Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: unsupported transaction error fallback #823

Merged
merged 5 commits into from
Jul 22, 2024

Conversation

aforaleka
Copy link
Contributor

@aforaleka aforaleka commented Jul 17, 2024

context: it's possible that the stringkey we pass from Abacus might not exist in localization, since we aren't translating protocol errors 1:1. example. other than just translating all error codes and catching all possible query results (which is not sustainable), we should fallback to show some error message that is relevant

  • correctly serialize transaction errors via stringifyTransactionError, because when a normal Error is thrown (e.g. query update failure), JSON.stringify will not work
  • update functions that takes in errorStringKey?: string to errorParams?: ErrorParams, the latter includes a potential error string key and a fallback error message
  • update string getter function to take a fallback param, so when the string key isn't valid (part of STRING_KEYS), we should explicitly define fallback string. this is kinda optional so open to thoughts!

before, query result failure (e.g. failure during subaccount transfer) will show no error strings. after:
image

Copy link

vercel bot commented Jul 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
v4-staging 🛑 Canceled (Inspect) Jul 22, 2024 7:32pm
v4-testnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2024 7:32pm

@aforaleka aforaleka force-pushed the handle-untranslated-error-fallback branch from 408e367 to 3719405 Compare July 17, 2024 18:54
@aforaleka aforaleka force-pushed the handle-untranslated-error-fallback branch from 3719405 to 377a6bb Compare July 17, 2024 19:42
@aforaleka aforaleka force-pushed the handle-untranslated-error-fallback branch from 9302e10 to 2becc0b Compare July 19, 2024 01:03
@aforaleka aforaleka force-pushed the handle-untranslated-error-fallback branch from 2becc0b to d86b896 Compare July 19, 2024 14:40
@aforaleka aforaleka marked this pull request as ready for review July 19, 2024 14:46
* Failed Query results (i.e., those starting with "Query failed") are matched via the error message, defaulting to "Unknown query result error".
* Unmatched errors will display the actual error message.
*/
export const stringifyTransactionError = (error: any): string => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[q] does broadcasterror cover all abacus cases? i'm curious as to whether or not this will also fix the onboarding/offboarding modal errors

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is where transaction errors are parsed!

that specific error in the slack channel is a broadcast error, so i think it's just because it wasn't passed through that function

@@ -375,7 +376,7 @@ const useSubaccountContext = ({ localDydxWallet }: { localDydxWallet?: LocalWall
if (success) {
onSuccess?.(data);
} else {
onError?.({ errorStringKey: parsingError?.stringKey });
onError?.(getValidParsingErrorParams(parsingError));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] should this function be named getValidOnErrorParamsFromParsingError? since what we're getting is the onErrorParams from the parsing error?

@aforaleka aforaleka force-pushed the handle-untranslated-error-fallback branch from fe28a1b to d4125e2 Compare July 22, 2024 19:25
@aforaleka aforaleka enabled auto-merge (squash) July 22, 2024 19:25
@aforaleka aforaleka disabled auto-merge July 22, 2024 19:26
@aforaleka aforaleka enabled auto-merge (squash) July 22, 2024 19:28
@aforaleka aforaleka merged commit b476b7c into main Jul 22, 2024
6 checks passed
@aforaleka aforaleka deleted the handle-untranslated-error-fallback branch July 22, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants