Skip to content

Additional fixes to 0.9 branch - #579

Open
Montoya wants to merge 17 commits into
mainfrom
cm/starknet-breaking-changes-followup
Open

Additional fixes to 0.9 branch#579
Montoya wants to merge 17 commits into
mainfrom
cm/starknet-breaking-changes-followup

Conversation

@Montoya

@Montoya Montoya commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Error handling and reliability fixes for the companion dapp and Snap

While testing the starknet.js v9 changes locally, snap RPC failures surfaced in the dapp as [object Object] with no usable stack, hiding several real bugs behind unreadable errors. This PR fixes the error reporting on both sides, plus three bugs the improved reporting uncovered.

wallet-ui: readable errors instead of [object Object]

  • New utils/error.ts with normalizeSnapError() / SnapRpcError: MetaMask rejects provider.request with a plain {code, message, data} object, which CRA's dev overlay renders as [object Object]. Errors are now rewrapped as real Errors with a readable message (including data.walletRpcError.code), preserving code/data so existing checks (err.code === 4100, isUserDenyError) still work.
  • useSnap.ts: invokeSnap, getInstalledSnaps, and requestSnap rethrow normalized errors tagged with the RPC method name.
  • App.tsx: the initSnap / checkConnection / initWalletData effects no longer leak unhandled promise rejections; failures log and show a toast.
  • useStarkNetSnap.ts:
    • isSnapRequireUpdate() moved inside initSnap's try block (was an unhandled-rejection site).
    • initWalletData clears the loader in finally so failures don't strand the loading backdrop.
    • The -32603 force-reinstall modal is skipped for local: snaps, since the snap returns -32603 for every internal failure and this falsely told local developers to "install the latest version".

starknet-snap: three bugs

  1. Original errors were discarded (index.tsx): the onRpcRequest catch-all wrapped unknown errors in UnknownError('Unable to execute the rpc request') and logged only the wrapper, making root causes unrecoverable. The original stack is now logged, and outside SNAP_ENV=prod the original message is appended to the client-visible error.

  2. Cross-request network race (chain-rpc-controller.ts): RPC controllers are module-level singletons, but preExecute stored the resolved network on this.network. Concurrent requests for different chains (e.g. switching networks quickly in the dapp) raced on that field and could execute with each other's network. Executions are now serialized per controller instance with a mutex. Verified no nested execute calls, so no deadlock; lock ordering (instance mutex → global state mutex) is consistent.

  3. Invalid RPC response shapes crashed discovery (utils/contract.ts): inside the sandbox (fetch proxied by endowment:network-access), contract call responses intermittently arrive with neither result nor error; starknet.js passes this through as undefined and AccountContractReader's resp[0] threw TypeError: Cannot read properties of undefined (reading '0') — reproducible on fresh install and on every network switch. The same requests replayed outside the sandbox (720 direct RPC calls, 80 via an identical starknet.js Provider) consistently returned well-formed bodies, so the corruption is transient and transport-specific. ContractReader.callContract now validates the response shape, retries up to 3× with backoff (logging each bad attempt's raw body), and throws a descriptive ContractReadError if exhausted. Genuine RPC errors (Contract not found for undeployed accounts) still throw immediately without retry.

Testing

  • tsc --noEmit and eslint clean on both packages.
  • Existing jest suites for the touched code pass (get-current-account, switch-network, contract, reader, discovery — 44 tests).
  • Manually verified against a fresh snap install with an undeployed account: previously failed with [object Object] → now initializes cleanly, with transient transport failures retried and named when they persist.

Note

Medium Risk
Touches core Snap RPC execution, network-scoped state, and wallet bootstrap paths; behavior changes are targeted but affect concurrent chain operations and failure UX.

Overview
Improves error visibility in the companion dapp and reliability in the Snap after starknet.js v9 testing surfaced failures as [object Object].

wallet-ui adds normalizeSnapError / SnapRpcError so MetaMask’s plain JSON-RPC rejections become real Errors with readable messages while keeping code and data. useSnap normalizes failures from invoke/install flows; App catches async init/check/load rejections and toasts them. initSnap moves version checks inside try, initWalletData always clears the loader in finally, and the -32603 force-reinstall modal is skipped for local: snaps.

starknet-snap logs the original error before wrapping unknown RPC failures, and in non-prod includes the underlying message in the client error. Chain RPC controllers serialize execute with a per-singleton mutex so concurrent requests cannot race on this.network. Contract reads retry transient invalid/empty RPC shapes (up to 3× with backoff) instead of crashing discovery with undefined[0].

Reviewed by Cursor Bugbot for commit a39a52f. Bugbot is set up for automated code reviews on this repo. Configure here.

FrederikBolding and others added 17 commits August 12, 2026 15:27
This surfaces errors to the React frontend better
…nses

Rapidly switching networks in the companion dapp caused
starkNet_getCurrentAccount to fail with "TypeError: Cannot read
properties of undefined (reading '0')" and could serve wrong-chain data.

Two fixes:

- ChainRpcController: all RPC controllers are module-level singletons,
  but preExecute stored the resolved network on `this.network`. Two
  concurrent requests for different chains raced on that field, so a
  request could execute against the other request's network. Executions
  are now serialized per controller instance with a mutex.

- ContractReader.callContract: starknet.js's fetchEndpoint resolves to
  undefined when a node responds without a `result` or `error` field,
  which crashed callers indexing `resp[0]` (AccountContractReader).
  Validate the response is a non-empty array and throw a descriptive
  ContractReadError (including entrypoint, address, and raw response)
  instead. Also guard `error.message` access for non-Error throws.
Inside the Snap sandbox, `fetch` is proxied across the sandbox boundary
by `endowment:network-access`, and account discovery issues several
concurrent contract calls through it. Intermittently a response arrives
with neither `result` nor `error`, which starknet.js's fetchEndpoint
passes through as `undefined`. The same requests replayed outside the
sandbox (720 direct RPC calls, 80 via an identical starknet.js
Provider) consistently return well-formed bodies, so the malformed
shape is transient and specific to the sandboxed transport.

ContractReader.callContract now retries up to 3 times with backoff when
the response shape is invalid, logging each bad attempt's raw response.
Genuine RPC errors (e.g. "Contract not found" for undeployed accounts)
are not retried and still throw immediately. Exhausting retries throws
a descriptive ContractReadError instead of the previous
"TypeError: Cannot read properties of undefined (reading '0')".
@Montoya
Montoya requested review from a team as code owners August 13, 2026 16:03
@Montoya
Montoya requested review from Julink-eth and jonesho and removed request for a team August 13, 2026 16:03
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed for 'consensys_starknet-snap-wallet-ui'

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed for 'consensys_starknet-snap-starknet-snap'

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
19.0% Coverage on New Code
9.9% Duplication on New Code

See analysis details on SonarQube Cloud

Base automatically changed from fb/starknet-breaking-changes to main August 18, 2026 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants