fix: parse jupiter errors to human readable translations #8433

NeOMakinG · 2024-12-20T15:47:00Z

Description

Added some error arrays to the swappers so every swappers can parse their errors into human readable errors

Issue (if applicable)

closes #8303

Risk

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Testing

Try to trade a very small amount of SOL to USDC on Solana with 0.01 as slippage
Sometime you can see the Trade consumed more fees than expected error too, we already recently upgraded the buffer for this, I suspect small amount trades could be the root cause but if not, we might need to investigate a bit this particular case

Engineering

n/a

Operations

n/a

🏁 My feature is behind a flag and doesn't require operations testing (yet)

Screenshots (if applicable)

NeOMakinG · 2024-12-20T16:09:39Z

packages/swapper/src/swappers/baseErrors.ts

+  SLIPPAGE_EXCEEDED = 'trade.errors.slippageExceeded',
+  UNDER_MINIMUM_AMOUNT = 'trade.errors.underMinimumAmount',
+  CONSUMED_MORE_FEES = 'trade.errors.consumedMoreFees',


It can be weird to have translation keys in the swapper package, but for the sake of simplicity, I wanted to keep it here

The very clean way would be to return error codes or strings, and map these errors codes/strings to our translation keys on react app side, but the swapper is not consumed outside of this scope

Depending on reviews args, we might want to do it, but my choice has been done already 😄

For the sake of simplicity, would actually prefer to keep just indexes. This is redeclaring keys from JUPITER_ERROR_VALUES, reading this is a big indirection for what should be a simple mapping (three count between JUPITER_ERROR_VALUES, redundant JUPITER_ERRORS, and this, can pattern match in /useTradeExecution instead

gomesalexandre · 2024-12-20T18:40:52Z

packages/swapper/src/swappers/JupiterSwapper/errors.ts

+import { SWAPPER_ERRORS } from '../baseErrors'
+
+export const JUPITER_ERROR_VALUES = {
+  SLIPPAGE_EXCEEDED: 'SlippageToleranceExceeded',


Suggested change

SLIPPAGE_EXCEEDED: 'SlippageToleranceExceeded',

SLIPPAGE_TOLERANCE_EXCEEDED: 'SlippageToleranceExceeded',

Changed in 2de18a2

gomesalexandre · 2024-12-20T18:40:57Z

packages/swapper/src/index.ts

@@ -6,3 +6,5 @@ export * from './thorchain-utils'
 export * from './cowswap-utils'
 export * from './safe-utils'
 export * from './swappers/CowSwapper'
+export * from './swappers/JupiterSwapper'


Unused

Suggested change

export * from './swappers/JupiterSwapper'

It's used again in 2de18a2

gomesalexandre · 2024-12-20T18:43:28Z

packages/swapper/src/swappers/JupiterSwapper/errors.ts

+import type { SwapperErrorMapping } from '../baseErrors'
+import { SWAPPER_ERRORS } from '../baseErrors'
+
+export const JUPITER_ERROR_VALUES = {


Not sure about the naming here, this is rather a subtring check , maybe something like pattern vernacular would be a better fit?

This also looks better suited to an enum.

Changed in 17f168a

gomesalexandre · 2024-12-20T18:48:36Z

packages/swapper/src/swappers/JupiterSwapper/errors.ts

+  CONSUMED_MORE_FEES: 'exceeded CUs meter',
+} as const
+
+export const JUPITER_ERRORS: SwapperErrorMapping[] = [


preferably-blocking: This is redundant.

this is a K,V mapping which 1/1 maps the keys and values of JUPITER_ERROR_VALUES

We're effectively reinventing the wheel and creating a poor man's flavour of Object

Changed in 17f168a

gomesalexandre · 2024-12-20T18:53:20Z

packages/swapper/src/swappers/baseErrors.ts

+export type SwapperErrorMapping = {
+  key: SWAPPER_ERRORS
+  value: string
+}


See https://github.com/shapeshift/web/pull/8433/files#r1894277094, we're effectively reinventing objects.

Yeah this is just a Record<SWAPPER_ERRORS, string>.

Changed in 17f168a

gomesalexandre · 2024-12-20T18:56:50Z

src/components/MultiHopTrade/components/MultiHopTradeConfirm/hooks/useTradeExecution.tsx

+            const swapperError = swapperErrors.find(error => e.message.includes(error.value))
+
+            if (swapperError) {
+              return {
+                message: translate(swapperError.key),
+                translationKey: swapperError.key,
+                error: undefined,
+              }
+            }
+
+            return {
+              message: translate(e.metadata.translation, e.metadata.options),
+              translationKey: e.metadata.translation,
+              error: e,
+            }
+          }
+
+          return {
+            message: (e as Error).message ?? undefined,
+            translationKey: undefined,
+            error: e,


suggestion:
switch(true) {} then case: <pattern>.test(e.message) and default to l.155`

Not relevant anymore, changed in 17f168a

gomesalexandre · 2024-12-20T18:58:57Z

packages/swapper/src/swappers/errors.ts

suggestion: filename is confusing, wouldn't be sure what errors means and the amount of nesting doesn't help here , maybe pattern terminology here too?

This file doesn't exists anymore, but changed to errorPatterns on jup side in 17f168a

gomesalexandre · 2024-12-20T18:59:25Z

packages/swapper/src/swappers/baseErrors.ts

Also if we use indexes this goes away, feels weird to import and export this guy in the same file (errors.ts)

Removed in 17f168a

gomesalexandre · 2024-12-20T18:59:59Z

packages/swapper/src/swappers/errors.ts

+import { SWAPPER_ERRORS } from './baseErrors'
+import { JUPITER_ERRORS } from './JupiterSwapper/errors'
+
+export { SWAPPER_ERRORS }


see https://github.com/shapeshift/web/pull/8433/files#r1894285675 re: import/export in the same file, feels like something we may not want to do?

Not relevant anymore, changed in 17f168a

gomesalexandre · 2024-12-20T19:08:22Z

packages/swapper/src/swappers/JupiterSwapper/errors.ts

+import { SWAPPER_ERRORS } from '../baseErrors'
+
+export const JUPITER_ERROR_VALUES = {
+  SLIPPAGE_EXCEEDED: 'SlippageToleranceExceeded',


q: do we even feel like errors at execution time is something that generalizes to the point we shouls abstract this away? Feels like all we're doing here could be done wherever the error bubbles up (useTradeExecution())

From what I'm seeing, this is really solana/jupiter specific, and none of our other supported chains would return errors at Tx broadcast time, are you aware of other Solana swappers doing the same as jupiter @NeOMakinG ?

The way we've done it is now throwing specific error types from the swapper easing the handling and seems like the right home, it's also a good pattern we could take in the future, wdyt?

Assuming we want to go from broadcast error to generalized error monad, 100%!

gomesalexandre · 2024-12-20T19:36:50Z

src/components/MultiHopTrade/components/MultiHopTradeConfirm/hooks/useTradeExecution.tsx

@@ -132,18 +133,37 @@ export const useTradeExecution = (
      dispatch(tradeQuoteSlice.actions.setSwapTxPending({ hopIndex, id: confirmedTradeId }))

      const onFail = (e: unknown) => {
-        const message = (() => {
+        const { message, translationKey, error } = (() => {
          if (e instanceof ChainAdapterError) {


Another suggestion which would help achieving what we want to here (keep error-handling at broadcast time colocated with its swapper): solana's executeSolanaTransaction is currently just consuming the base implementation of executeSolanaTransaction from swapper/src/utils.

If you wrap executeSolanaTx into an handler, then you can actually do proper error-handling there and return something better

diff --git a/packages/swapper/src/swappers/JupiterSwapper/JupiterSwapper.ts b/packages/swapper/src/swappers/JupiterSwapper/JupiterSwapper.ts index eb1966aead..deeba7c500 100644 --- a/packages/swapper/src/swappers/JupiterSwapper/JupiterSwapper.ts +++ b/packages/swapper/src/swappers/JupiterSwapper/JupiterSwapper.ts @@ -1,0 +2 @@ import type { AssetId } from '@shapeshiftoss/caip' +import { ChainAdapterError } from '@shapeshiftoss/chain-adapters' @@ -7,0 +9,7 @@ import { jupiterSupportedChainIds } from './utils/constants' +export class SolanaLogsError extends Error { + constructor(message: string) { + super(message) + this.name = 'SolanaLogsError' + } +} + @@ -9 +17,16 @@ export const jupiterSwapper: Swapper = { - executeSolanaTransaction, + executeSolanaTransaction: async (...args) => { + try { + const txid = await executeSolanaTransaction(...args) + return txid + } catch (e: unknown) { + if (e instanceof ChainAdapterError) { + // replace me with actual pattern matching + const isLogsError = true + // where errorMember is the actual error from enum + if (isLogsError) throw new SolanaLogsError(SLIPPAGE_EXCEEDED) + } + + // rethrow is not instanceof ChainAdapterError, shouldn't be the case but jus to satisfy TS + throw e + } + }, diff --git a/src/components/MultiHopTrade/components/MultiHopTradeConfirm/hooks/useTradeExecution.tsx b/src/components/MultiHopTrade/components/MultiHopTradeConfirm/hooks/useTradeExecution.tsx index c87f2e4589..ea4894c0e1 100644 --- a/src/components/MultiHopTrade/components/MultiHopTradeConfirm/hooks/useTradeExecution.tsx +++ b/src/components/MultiHopTrade/components/MultiHopTradeConfirm/hooks/useTradeExecution.tsx @@ -24,0 +25 @@ import { LIFI_TRADE_POLL_INTERVAL_MILLISECONDS } from '@shapeshiftoss/swapper/di +import { SolanaLogsError } from '@shapeshiftoss/swapper/src/swappers/JupiterSwapper/JupiterSwapper' @@ -136,0 +138,3 @@ export const useTradeExecution = ( + if (e instanceof SolanaLogsError) { + // switch error enum member to translation here + }

Implemented in 17f168a

gomesalexandre

Wasn't able to get to any error state as Jupiter swaps are turbos stable now but conceptually looks like it should do what it says on the box.

However nothing a bit of confusing indirection and data structures looking off here.

Suggestion:

Keep things atomic and use indexes only (as possible)
Don't colocate translations keys in swapper, which would help with the former (just return an error from enum)
Pattern match in web using switch (true) { case (<pattern>.test(str)): } (or, if returning

Alternatively, proposed to re-throw a SolanaLogsError, which would avoid this altogether

NeOMakinG · 2024-12-23T12:19:36Z

8ed7c3b

Found a better home for translation keys in 8ed7c3b

Should be ready for review again!

gomesalexandre · 2024-12-23T15:49:54Z

packages/swapper/src/swappers/JupiterSwapper/JupiterSwapper.ts

+      const txid = await executeSolanaTransaction(...args)
+      return txid
+    } catch (e) {
+      if (e instanceof Error) {


remove me, all errors are instanceof Error

gomesalexandre · 2024-12-23T15:53:29Z

src/assets/translations/en/main.json

@@ -974,7 +974,10 @@
      "unsafeQuote": "This amount is below the recommended minimum for this pair (%{recommendedMin} %{symbol}). This could cause your trade to fail or loss of funds.",
      "rateLimitExceeded": "Rate limit exceeded, try again later",
      "maxSlippageExceededWithExpectedSlippage": "The maximum allowed slippage tolerance for this trade has been exceeded during simulation. Expected slippage: %{expectedSlippage}%. Try again with a higher slipppage tolerance.",
-      "executionRevertedWithExpectedSlippage": "Execution reverted during execution. This may be due to insufficient slippage tolerance. Expected slippage: %{expectedSlippage}%. Try again with a higher slipppage tolerance."
+      "executionRevertedWithExpectedSlippage": "Execution reverted during execution. This may be due to insufficient slippage tolerance. Expected slippage: %{expectedSlippage}%. Try again with a higher slipppage tolerance.",
+      "slippageExceeded": "Slippage tolerance exceeded.",


Suggested change

"slippageExceeded": "Slippage tolerance exceeded.",

"slippageToleranceExceeded": "Slippage tolerance exceeded.",

gomesalexandre · 2024-12-23T15:55:17Z

packages/swapper/src/swappers/JupiterSwapper/errorPatterns.ts

+export enum JUPITER_ERROR_PATTERNS {
+  SLIPPAGE_TOLERANCE_EXCEEDED = 'SlippageToleranceExceeded',
+  UNDER_MINIMUM_AMOUNT = 'RequireGteViolated',
+  CONSUMED_MORE_FEES = 'exceeded CUs meter',
+}
+
+export enum JUPITER_ERROR_NAMES {
+  SLIPPAGE_TOLERANCE_EXCEEDED = 'SlippageToleranceExceeded',
+  UNDER_MINIMUM_AMOUNT = 'UnderMinimumAmount',
+  CONSUMED_MORE_FEES = 'ConsumedMoreFees',
+}


nitpick: don't pluralize enums

gomesalexandre · 2024-12-23T15:59:17Z

src/constants/errorMapping.ts

nitpick: <error> in values of JUPITER_ERROR_NAMES are the same as trade.errors.<error> (except for SlippageToleranceExceeded, see https://github.com/shapeshift/web/pull/8433/files#r1895898762) with a different casing only, we could use lodash/camelCase to avoid another mapping

gomesalexandre

Much cleaner now! Added some additional cleanup comments to tackle at your own convenience, but happy with this one conceptually as well as runtime-wise:

https://jam.dev/c/8203fb6a-d518-453c-8a0c-bda5d6d40668

NeOMakinG added 2 commits December 20, 2024 16:15

fix: parse jupiter errors

59e9a58

fix: review feedbacks

e2ee21d

NeOMakinG commented Dec 20, 2024

View reviewed changes

NeOMakinG marked this pull request as ready for review December 20, 2024 16:09

NeOMakinG requested a review from a team as a code owner December 20, 2024 16:09

gomesalexandre self-requested a review December 20, 2024 16:37

gomesalexandre reviewed Dec 20, 2024

View reviewed changes

NeOMakinG added 4 commits December 21, 2024 01:30

fix: review feedbacks

98d42b4

fix: review feedbacks

2de18a2

fix: review feedback

17f168a

fix: review feedback

8ed7c3b

gomesalexandre reviewed Dec 23, 2024

View reviewed changes

gomesalexandre approved these changes Dec 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: parse jupiter errors to human readable translations #8433

fix: parse jupiter errors to human readable translations #8433

NeOMakinG commented Dec 20, 2024

NeOMakinG Dec 20, 2024

gomesalexandre Dec 20, 2024 •

edited

Loading

gomesalexandre Dec 20, 2024

NeOMakinG Dec 21, 2024

gomesalexandre Dec 20, 2024

NeOMakinG Dec 21, 2024

gomesalexandre Dec 20, 2024

0xApotheosis Dec 22, 2024

NeOMakinG Dec 23, 2024

gomesalexandre Dec 20, 2024 •

edited

Loading

NeOMakinG Dec 23, 2024

gomesalexandre Dec 20, 2024

0xApotheosis Dec 22, 2024

NeOMakinG Dec 23, 2024

gomesalexandre Dec 20, 2024 •

edited

Loading

NeOMakinG Dec 23, 2024

gomesalexandre Dec 20, 2024

NeOMakinG Dec 23, 2024

gomesalexandre Dec 20, 2024

NeOMakinG Dec 23, 2024

gomesalexandre Dec 20, 2024

NeOMakinG Dec 23, 2024

gomesalexandre Dec 20, 2024

NeOMakinG Dec 23, 2024

gomesalexandre Dec 23, 2024

gomesalexandre Dec 20, 2024 •

edited

Loading

NeOMakinG Dec 23, 2024

gomesalexandre left a comment •

edited

Loading

NeOMakinG commented Dec 23, 2024 •

edited

Loading

gomesalexandre Dec 23, 2024

gomesalexandre Dec 23, 2024 •

edited

Loading

gomesalexandre Dec 23, 2024

gomesalexandre Dec 23, 2024 •

edited

Loading

gomesalexandre left a comment

	SLIPPAGE_EXCEEDED: 'SlippageToleranceExceeded',
	SLIPPAGE_TOLERANCE_EXCEEDED: 'SlippageToleranceExceeded',

	"slippageExceeded": "Slippage tolerance exceeded.",
	"slippageToleranceExceeded": "Slippage tolerance exceeded.",

fix: parse jupiter errors to human readable translations #8433

Are you sure you want to change the base?

fix: parse jupiter errors to human readable translations #8433

Conversation

NeOMakinG commented Dec 20, 2024

Description

Issue (if applicable)

Risk

Testing

Engineering

Operations

Screenshots (if applicable)

Choose a reason for hiding this comment

gomesalexandre Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gomesalexandre Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gomesalexandre Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gomesalexandre Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gomesalexandre left a comment • edited Loading

Choose a reason for hiding this comment

NeOMakinG commented Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

gomesalexandre Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gomesalexandre Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

gomesalexandre left a comment

Choose a reason for hiding this comment

gomesalexandre Dec 20, 2024 •

edited

Loading

gomesalexandre Dec 20, 2024 •

edited

Loading

gomesalexandre Dec 20, 2024 •

edited

Loading

gomesalexandre Dec 20, 2024 •

edited

Loading

gomesalexandre left a comment •

edited

Loading

NeOMakinG commented Dec 23, 2024 •

edited

Loading

gomesalexandre Dec 23, 2024 •

edited

Loading

gomesalexandre Dec 23, 2024 •

edited

Loading