From 06508eb3a671ce8fdf55d1ae3dbe8849e17e40a9 Mon Sep 17 00:00:00 2001 From: Alex Connolly Date: Fri, 7 Aug 2026 18:29:43 +1000 Subject: [PATCH] fix(sdk): promote three bug-catching lint rules to error and fix the fallout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #2944. That bug shipped because the rule which would have caught it was set to `warn`, so nothing failed CI. This promotes three rules that catch runtime defects rather than style preferences, and clears the codebase of them so the gate stays green. Promoted to error: - typescript/no-non-null-asserted-optional-chain — `a?.b!` is undefined at runtime while typed as present - typescript/no-base-to-string — stringifying an object yields "[object Object]", silently gutting error messages and analytics payloads - eslint/preserve-caught-error — rethrowing without `cause` discards the original stack Two real defects surfaced by this: - tokenBridge.getFee passed `async () => {...}` to Promise.all without invoking it, so Promise.all resolved the function object and the chain-id validation never ran. Note this restores validation that has been dead: callers passing mismatched chain ids will now get the intended error. - PayWithCoins reported sale failures via `error.toString()` on a SignOrderError ({ type, data }), so every failure event carried the literal string "[object Object]" instead of the failure type. Also fixed: an undefined provider could reach NetworkSwitchDrawer via `from?.browserProvider!`; OrderSummary dereferenced a possibly-missing smartCheckoutResult; SaleWidget built block-explorer links containing "undefined"; fundingBalanceFees pushed fees with a missing required token. `oxlint --fix` was not used. On this repo it rewrites `.sort()` to `.toSorted()` (ES2023, while tsconfig targets ES2022), drops entries from React dependency arrays, and leaves dead whitespace. Every change here is hand-written. Deferred, with counts, to keep this reviewable: - no-floating-promises (230) — the rule closest to #2944's root cause. Needs per-site judgment (`void` for fire-and-forget, real handling for user-triggered actions), so it gets its own PR. - no-unsafe-enum-comparison (93) — needs a canonical type per comparison - react-hooks/exhaustive-deps (235) — every fix changes render behaviour Do not enable the `radix` rule. It is off (style category), and its autofix is what caused #2944. Co-Authored-By: Claude Opus 5 --- .oxlintrc.json | 6 ++- .../src/lib/connectEIP6963Provider.ts | 3 +- .../bridge/components/BridgeReviewSummary.tsx | 38 +++++++++++-------- .../src/widgets/sale/SaleWidget.tsx | 10 +++-- .../sale/functions/fundingBalanceFees.ts | 8 ++-- .../src/widgets/sale/views/OrderSummary.tsx | 4 +- .../src/widgets/sale/views/PayWithCoins.tsx | 6 ++- .../src/widgets/wallet/WalletWidgetRoot.tsx | 4 +- .../bridge/sdk/src/lib/validation.test.ts | 8 ++-- .../internal/bridge/sdk/src/tokenBridge.ts | 9 +++-- .../internal/dex/sdk/src/lib/utils.test.ts | 2 +- packages/orderbook/src/orderbook.ts | 4 +- packages/wallet/src/magic/magicTEESigner.ts | 2 +- packages/wallet/src/zkEvm/relayerClient.ts | 2 +- 14 files changed, 64 insertions(+), 42 deletions(-) diff --git a/.oxlintrc.json b/.oxlintrc.json index 55c955dc71..9c06ea12ad 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -66,6 +66,8 @@ } ], + "eslint/preserve-caught-error": "error", + "eslint/no-unused-vars": [ "warn", { @@ -80,7 +82,7 @@ "typescript/restrict-template-expressions": "warn", "typescript/await-thenable": "warn", "typescript/no-redundant-type-constituents": "warn", - "typescript/no-non-null-asserted-optional-chain": "warn", + "typescript/no-non-null-asserted-optional-chain": "error", "typescript/no-duplicate-type-constituents": "warn", "typescript/no-unsafe-type-assertion": "off", "typescript/no-unnecessary-type-assertion": "off", @@ -90,7 +92,7 @@ "typescript/no-duplicate-enum-values": "warn", "typescript/no-unnecessary-parameter-property-assignment": "off", "typescript/no-this-alias": "warn", - "typescript/no-base-to-string": "warn", + "typescript/no-base-to-string": "error", "typescript/no-wrapper-object-types": "warn", "typescript/no-for-in-array": "warn", diff --git a/packages/checkout/widgets-lib/src/lib/connectEIP6963Provider.ts b/packages/checkout/widgets-lib/src/lib/connectEIP6963Provider.ts index ffb09181ce..ae6e67829c 100644 --- a/packages/checkout/widgets-lib/src/lib/connectEIP6963Provider.ts +++ b/packages/checkout/widgets-lib/src/lib/connectEIP6963Provider.ts @@ -43,9 +43,10 @@ export const connectEIP6963Provider = async ( case CheckoutErrorType.USER_REJECTED_REQUEST_ERROR: throw new Error( ConnectEIP6963ProviderError.USER_REJECTED_REQUEST_ERROR, + { cause: error }, ); default: - throw new Error(ConnectEIP6963ProviderError.CONNECT_ERROR); + throw new Error(ConnectEIP6963ProviderError.CONNECT_ERROR, { cause: error }); } } }; diff --git a/packages/checkout/widgets-lib/src/widgets/bridge/components/BridgeReviewSummary.tsx b/packages/checkout/widgets-lib/src/widgets/bridge/components/BridgeReviewSummary.tsx index 149eafa8ab..ac8ea909af 100644 --- a/packages/checkout/widgets-lib/src/widgets/bridge/components/BridgeReviewSummary.tsx +++ b/packages/checkout/widgets-lib/src/widgets/bridge/components/BridgeReviewSummary.tsx @@ -305,24 +305,28 @@ export function BridgeReviewSummary() { }, []); const handleNetworkSwitch = useCallback((provider: WrappedBrowserProvider) => { + // Both sides must already be selected — this only ever runs after the review + // screen has them. Bail rather than dispatching undefined into bridge state. + if (!from || !to) return; + bridgeDispatch({ payload: { type: BridgeActions.SET_WALLETS_AND_NETWORKS, from: { browserProvider: provider, - walletAddress: from?.walletAddress!, - walletProviderInfo: from?.walletProviderInfo!, - network: from?.network!, + walletAddress: from.walletAddress, + walletProviderInfo: from.walletProviderInfo, + network: from.network, }, to: { - browserProvider: to?.browserProvider!, - walletAddress: to?.walletAddress!, - walletProviderInfo: to?.walletProviderInfo!, - network: to?.network!, + browserProvider: to.browserProvider, + walletAddress: to.walletAddress, + walletProviderInfo: to.walletProviderInfo, + network: to.network, }, }, }); - }, [from?.browserProvider, from?.network, to?.browserProvider, to?.network]); + }, [from, to]); useEffect(() => { if (!from?.browserProvider) return; @@ -627,14 +631,16 @@ export function BridgeReviewSummary() { )} - setShowSwitchNetworkDrawer(false)} - onNetworkSwitch={handleNetworkSwitch} - /> + {from && ( + setShowSwitchNetworkDrawer(false)} + onNetworkSwitch={handleNetworkSwitch} + /> + )} )} {viewState.view.type === SaleWidgetViews.ORDER_SUMMARY && ( diff --git a/packages/checkout/widgets-lib/src/widgets/sale/functions/fundingBalanceFees.ts b/packages/checkout/widgets-lib/src/widgets/sale/functions/fundingBalanceFees.ts index 1e160430b0..d60970ff7d 100644 --- a/packages/checkout/widgets-lib/src/widgets/sale/functions/fundingBalanceFees.ts +++ b/packages/checkout/widgets-lib/src/widgets/sale/functions/fundingBalanceFees.ts @@ -87,8 +87,10 @@ export const getFundingBalanceFeeBreakDown = ( } const addFee = (fee: Fee, label: string, prefix: string = '~ ') => { - if (fee.amount > 0) { - const formattedFee = formatUnits(fee.amount, fee?.token?.decimals); + // A fee without a token can't be rendered — FormattedFee.token is required + // and consumers read token.symbol, so skip rather than push a hole. + if (fee.amount > 0 && fee.token) { + const formattedFee = formatUnits(fee.amount, fee.token.decimals); feesBreakdown.push({ label, @@ -103,7 +105,7 @@ export const getFundingBalanceFeeBreakDown = ( )}`, amount: `${tokenValueFormat(formattedFee)}`, prefix, - token: fee?.token!, + token: fee.token, }); } }; diff --git a/packages/checkout/widgets-lib/src/widgets/sale/views/OrderSummary.tsx b/packages/checkout/widgets-lib/src/widgets/sale/views/OrderSummary.tsx index 27a19eb4ff..49b2519845 100644 --- a/packages/checkout/widgets-lib/src/widgets/sale/views/OrderSummary.tsx +++ b/packages/checkout/widgets-lib/src/widgets/sale/views/OrderSummary.tsx @@ -231,7 +231,9 @@ export function OrderSummary({ subView }: OrderSummaryProps) { // suggest to top up base currency balance const smartCheckoutResult = fundingBalancesResult.find( (result) => result.currency.base, - )?.smartCheckoutResult!; + )?.smartCheckoutResult; + if (!smartCheckoutResult) return; + const data = getTopUpViewData( smartCheckoutResult.transactionRequirements, ); diff --git a/packages/checkout/widgets-lib/src/widgets/sale/views/PayWithCoins.tsx b/packages/checkout/widgets-lib/src/widgets/sale/views/PayWithCoins.tsx index 0c82cacf5f..1e035e11b1 100644 --- a/packages/checkout/widgets-lib/src/widgets/sale/views/PayWithCoins.tsx +++ b/packages/checkout/widgets-lib/src/widgets/sale/views/PayWithCoins.tsx @@ -89,7 +89,11 @@ export function PayWithCoins() { }, (error, txns) => { const details = { transactionId: signResponse?.transactionId }; - sendFailedEvent(error.toString(), error, txns, undefined, details); // checkoutPrimarySalePaymentMethods_FailEventFailed + // `error` is a SignOrderError ({ type, data }), not an Error. Its default + // toString is "[object Object]", so this event was reporting nothing + // useful — `type` is the field that identifies the failure. + const reason = error instanceof Error ? error.message : error.type; + sendFailedEvent(reason, error, txns, undefined, details); // checkoutPrimarySalePaymentMethods_FailEventFailed goToErrorView(error.type, error.data); }, onTxnStepExecuteAll, diff --git a/packages/checkout/widgets-lib/src/widgets/wallet/WalletWidgetRoot.tsx b/packages/checkout/widgets-lib/src/widgets/wallet/WalletWidgetRoot.tsx index 42b8051fea..3bdc59beb6 100644 --- a/packages/checkout/widgets-lib/src/widgets/wallet/WalletWidgetRoot.tsx +++ b/packages/checkout/widgets-lib/src/widgets/wallet/WalletWidgetRoot.tsx @@ -96,8 +96,8 @@ export class Wallet extends Base { config={this.strongConfig()} walletConfig={{ showDisconnectButton: - this.properties.config?.showDisconnectButton!, - showNetworkMenu: this.properties.config?.showNetworkMenu!, + this.properties.config?.showDisconnectButton ?? false, + showNetworkMenu: this.properties.config?.showNetworkMenu ?? false, }} /> diff --git a/packages/internal/bridge/sdk/src/lib/validation.test.ts b/packages/internal/bridge/sdk/src/lib/validation.test.ts index 07c87e8372..56c64f272d 100644 --- a/packages/internal/bridge/sdk/src/lib/validation.test.ts +++ b/packages/internal/bridge/sdk/src/lib/validation.test.ts @@ -35,7 +35,7 @@ describe('Validation', () => { try { await validateChainConfiguration(bridgeConfig); } catch (error: any) { - throw new Error(`Should not have thrown an error, but threw ${error}`); + throw new Error(`Should not have thrown an error, but threw ${error}`, { cause: error }); } }); @@ -107,7 +107,7 @@ describe('Validation', () => { try { await checkReceiver(tokenSent, destinationChainId, '0x123', config); } catch (error: any) { - throw new Error(`Should not have thrown an error, but threw ${error}`); + throw new Error(`Should not have thrown an error, but threw ${error}`, { cause: error }); } }); @@ -125,7 +125,7 @@ describe('Validation', () => { try { await checkReceiver(tokenSent, destinationChainId, '0x123', config); } catch (error: any) { - throw new Error(`Should not have thrown an error, but threw ${error}`); + throw new Error(`Should not have thrown an error, but threw ${error}`, { cause: error }); } }); @@ -147,7 +147,7 @@ describe('Validation', () => { await checkReceiver(tokenSent, destinationChainId, '0x123', config); expect(mockProvider.getCode).toHaveBeenCalledTimes(1); } catch (error: any) { - throw new Error(`Should not have thrown an error, but threw ${error}`); + throw new Error(`Should not have thrown an error, but threw ${error}`, { cause: error }); } }); diff --git a/packages/internal/bridge/sdk/src/tokenBridge.ts b/packages/internal/bridge/sdk/src/tokenBridge.ts index 318bff4606..f9ba69cd1e 100644 --- a/packages/internal/bridge/sdk/src/tokenBridge.ts +++ b/packages/internal/bridge/sdk/src/tokenBridge.ts @@ -158,11 +158,14 @@ export class TokenBridge { public async getFee(req: BridgeFeeRequest): Promise { const [, , res] = await Promise.all([ this.initialise(), - async () => { + // Note the trailing `()`. This was previously passed as an uninvoked async + // function, so Promise.all resolved it to the function object and the + // chain-id validation never ran. + (async () => { if (req.action !== BridgeFeeActions.FINALISE_WITHDRAWAL) { await validateChainIds(req.sourceChainId, req.destinationChainId, this.config); } - }, + })(), this.getFeePrivate(req), ]); return res; @@ -702,7 +705,7 @@ export class TokenBridge { const [allowance, feeData, tenderlyRes] = await Promise.all([ this.getAllowance(direction, token, sender), this.config.childProvider.getFeeData(), - await this.getDynamicWithdrawGasRootChain( + this.getDynamicWithdrawGasRootChain( direction.destinationChainId, sender, recipient, diff --git a/packages/internal/dex/sdk/src/lib/utils.test.ts b/packages/internal/dex/sdk/src/lib/utils.test.ts index 93214058ac..c0ea4e7f55 100644 --- a/packages/internal/dex/sdk/src/lib/utils.test.ts +++ b/packages/internal/dex/sdk/src/lib/utils.test.ts @@ -25,7 +25,7 @@ const provider = { if (payload.to === USDC_TEST_TOKEN.address) { return USDC_TEST_TOKEN.decimals.toString(16); } - throw new Error(`Unrecognized ERC20: ${payload.to}`); + throw new Error(`Unrecognized ERC20: ${JSON.stringify(payload.to)}`); } throw new Error(`Call not supported: ${payload.data}`); }), diff --git a/packages/orderbook/src/orderbook.ts b/packages/orderbook/src/orderbook.ts index fa4b663d94..42e9bf6800 100644 --- a/packages/orderbook/src/orderbook.ts +++ b/packages/orderbook/src/orderbook.ts @@ -94,7 +94,7 @@ export class Orderbook { if (config.overrides?.jsonRpcProviderUrl) { finalConfig.provider = getConfiguredProvider( - config.overrides?.jsonRpcProviderUrl!, + config.overrides.jsonRpcProviderUrl, config.baseConfig.rateLimitingKey, ); } @@ -778,7 +778,7 @@ export class Orderbook { if (orderResult.status.name !== OrderStatusName.ACTIVE) { throw new Error( - `Cannot fulfil order that is not active. Current status: ${orderResult.status}`, + `Cannot fulfil order that is not active. Current status: ${orderResult.status.name}`, ); } diff --git a/packages/wallet/src/magic/magicTEESigner.ts b/packages/wallet/src/magic/magicTEESigner.ts index 6efbeab5ac..465a181a56 100644 --- a/packages/wallet/src/magic/magicTEESigner.ts +++ b/packages/wallet/src/magic/magicTEESigner.ts @@ -235,7 +235,7 @@ export default class MagicTEESigner implements WalletSigner { errorMessage += `: ${(error as Error).message}`; } - throw new Error(errorMessage); + throw new Error(errorMessage, { cause: error }); } }, 'magicSignMessage'); } diff --git a/packages/wallet/src/zkEvm/relayerClient.ts b/packages/wallet/src/zkEvm/relayerClient.ts index 7af663f4b5..50879c6755 100644 --- a/packages/wallet/src/zkEvm/relayerClient.ts +++ b/packages/wallet/src/zkEvm/relayerClient.ts @@ -155,7 +155,7 @@ export class RelayerClient { } catch (parseError) { const preview = RelayerClient.getResponsePreview(responseText); // eslint-disable-next-line max-len - throw new Error(`Relayer JSON parse error: ${parseError instanceof Error ? parseError.message : 'Unknown error'}. Content: "${preview}"`); + throw new Error(`Relayer JSON parse error: ${parseError instanceof Error ? parseError.message : 'Unknown error'}. Content: "${preview}"`, { cause: parseError }); } if (jsonResponse.error) {