sneak/AutistMask
1
0
Fork 0
Code Issues 42 Pull Requests 8 Actions Packages Projects Releases Wiki Activity

fix: carry EIP-1193 error codes through to the page (closes #274) #278

Merged
clawbot merged 1 commits from fix/issue-274-eip1193-error-codes into next 2026-08-12 13:47:34 +02:00
Conversation 1 Commits 1 Files Changed 5 +394 -18
clawbot commented 2026-08-12 13:36:49 +02:00
Collaborator
Copy Link

Closes #274.

src/content/inpage.js rebuilt every failure as new Error(error.message || "Request failed"), dropping code. The code is produced correctly by the background and relayed intact by the content script — it was lost in the last hop, in the provider the page actually talks to. A dApp checking err.code === 4001 saw undefined, so a wallet the user deliberately declined was indistinguishable from a wallet that broke, and a well-behaved site shows an error or retries instead of accepting the refusal. RULES.md requires the code.

The error shape, and why

EIP-1193 specifies a ProviderRpcError with code, message and optional data. This defines the class rather than bolting properties onto a bare Error:

class ProviderRpcError extends Error {
    constructor(code, message, data) {
        super(message);
        this.name = "ProviderRpcError";
        this.code = code;
        if (data !== undefined) this.data = data;
    }
}

The object crosses no boundary after this point — it is constructed in the page's own realm and handed straight to the caller's catch — so the prototype survives, instanceof works for anything holding a reference to the class, and error.name is a stable thing for a dApp to branch on. A property bag on an Error would give the same code with none of that, for no saving.

Two deliberate limits on the shape:

  • Pass-through, not a whitelist. Whatever code arrived is carried verbatim rather than matched against a list of known values. A code the provider has never heard of is still the truth about what happened, and a code added to the background later must reach the page without editing this file. Covered by a test that feeds it 4900, which nothing emits today.
  • A code-less error stays a plain Error. Several background paths report {message} with no code. Those keep exactly today's behaviour, with no code property at all — not code: undefined. A ProviderRpcError whose code is undefined would advertise a conformance it does not have, and 'code' in err is exactly what a careful dApp asks. Asserted both ways.

message is untouched in every case, including the "Request failed" fallback for an error that arrives without one. Nothing the background produces changes.

Codes the background emits, and which now reach the page

Read out of src/background/index.js; every one of them now arrives on the page's Error, because the provider passes them through rather than enumerating them.

Code Where it is produced Reaches the page
4001 site connection denied or rejected at the prompt; tx and sign approvals rejected, closed by the window manager, or torn down on disconnect (7 sites) yes
4100 personal_sign / eth_sign / eth_signTypedData_v4 from a site that is not connected, or naming an address that is not the active one (6 sites) yes
4902 wallet_switchEthereumChain / wallet_addEthereumChain for a chain that is not Mainnet or Sepolia (2 sites) yes
none "No accounts available", "Unsupported method: …", a failed proxy RPC, a failed tx population, an address that changed mid-preparation unchanged: plain Error, same message

4200, 4900 and 4901 are named by EIP-1193 but are not produced anywhere in this codebase today, so nothing is invented for them; the pass-through means they need no change here if they ever are. The code-less row is untouched on purpose — this unit is confined to what the provider surfaces, not to what the background produces. That "Unsupported method" arguably wants 4200 is filed separately as #279.

Every request path

request, enable, send(method, params), send({method, params}) and sendAsync all funnel through the single AUTISTMASK_RESPONSE listener, so one conversion point covers all of them. tests/inpageErrors.test.js drives each entry point separately and asserts the code on each, so the claim is tested rather than argued.

Tests

tests/inpageErrors.test.js (new, 17 cases) loads the real src/content/inpage.js — a bare IIFE, not a module — by compiling it with its globals as function parameters, so nothing leaks between tests and the source compiles in the test's own realm, which is what makes expect(err).toBeInstanceOf(Error) meaningful. There is no jsdom in this repo; this follows the hand-rolled stub convention of tests/txStatus.test.js.

Not vacuous — with the inpage.js change stashed and the test file unchanged, 12 of the 17 fail:

✕ a user rejection arrives as code 4001
✕ it is a ProviderRpcError, and an Error
✕ 4100 unauthorized arrives intact
✕ 4902 unrecognized chain arrives intact
✕ a code the provider has never heard of is passed through
✕ data is carried when the boundary sent it
✓ no data property is invented when the boundary sent none
✓ a coded error keeps the message byte for byte
✓ an error the background sent with no code keeps its message
✓ an error with no code gets no code property at all
✕ an error with no message keeps the generic fallback
✕ request()
✕ enable()
✕ send(method, params)
✕ send({ method, params })
✕ sendAsync() hands the code to its callback
✓ a result still resolves
Tests: 12 failed, 5 passed, 17 total

The five that pass unchanged are the ones asserting behaviour this deliberately preserves: the untouched messages, and the absence of code/data where the boundary sent none. That is the point of having them.

The e2e probe, flipped

#273 landed four rejection cases that printed page Error.code=undefined page Error carries a code=false rather than asserting, so this unit could flip them on. assertUserRejection() in tests/e2e/run.js now requires the code on the page's Error as well as on the wire, and requires the ProviderRpcError name; the fixture in tests/e2e/network.js records name alongside the code and hasCode it already captured.

Failing before the provider change, with the flipped assertion in place — make test-e2e, exit 1:

not ok 29 - eth_requestAccounts rejected at the prompt returns a rejection (#183)
  eth_requestAccounts rejection reached the page as an error with no code property at all, so a dApp cannot tell the user's refusal from a failure: {"settled":"rejected","message":"User rejected the request.","name":"Error","hasCode":false}
not ok 32 - personal_sign rejected returns a rejection to the page (#183)
  personal_sign rejection reached the page as an error with no code property at all, so a dApp cannot tell the user's refusal from a failure: {"settled":"rejected","message":"User rejected the request.","name":"Error","hasCode":false}
not ok 34 - eth_signTypedData_v4 rejected returns a rejection to the page (#183)
  eth_signTypedData_v4 rejection reached the page as an error with no code property at all, so a dApp cannot tell the user's refusal from a failure: {"settled":"rejected","message":"User rejected the request.","name":"Error","hasCode":false}
not ok 36 - eth_sendTransaction rejected broadcasts nothing (#183)
  eth_sendTransaction rejection reached the page as an error with no code property at all, so a dApp cannot tell the user's refusal from a failure: {"settled":"rejected","message":"User rejected the request.","name":"Error","hasCode":false}
# 33/37 tests passed
# FAILED

Note the recorded "message":"User rejected the request." in the failing output above and in the passing run below: byte-identical, which is the message-unchanged claim shown rather than asserted in prose.

Passing after, on the rebased branch — make test-e2e, exit 0:

# eth_requestAccounts rejection: code 4001 on the wire and on the page's ProviderRpcError
# personal_sign rejection: code 4001 on the wire and on the page's ProviderRpcError
# eth_signTypedData_v4 rejection: code 4001 on the wire and on the page's ProviderRpcError
# eth_sendTransaction rejection: code 4001 on the wire and on the page's ProviderRpcError
# 37/37 tests passed

Verification

Both re-run after rebasing onto next at c755a5e.

  • make check — exit 0. 681 tests in 28 suites, test-verify-build: 18 case(s) passed, prettier clean.
  • make test-e2e — exit 0, 37/37, in the pinned Playwright container.

Scope

src/content/inpage.js and the tests only. src/content/index.js, src/background/index.js and src/popup/views/approval.js are untouched, so this does not collide with the work on #153. The window.close() accommodation the harness makes for #275 is left exactly as it was.

Closes https://git.eeqj.de/sneak/AutistMask/issues/274. `src/content/inpage.js` rebuilt every failure as `new Error(error.message || "Request failed")`, dropping `code`. The code is produced correctly by the background and relayed intact by the content script — it was lost in the last hop, in the provider the page actually talks to. A dApp checking `err.code === 4001` saw `undefined`, so a wallet the user deliberately declined was indistinguishable from a wallet that broke, and a well-behaved site shows an error or retries instead of accepting the refusal. `RULES.md` requires the code. ## The error shape, and why EIP-1193 specifies a `ProviderRpcError` with `code`, `message` and optional `data`. This defines the class rather than bolting properties onto a bare `Error`: ```js class ProviderRpcError extends Error { constructor(code, message, data) { super(message); this.name = "ProviderRpcError"; this.code = code; if (data !== undefined) this.data = data; } } ``` The object crosses no boundary after this point — it is constructed in the page's own realm and handed straight to the caller's `catch` — so the prototype survives, `instanceof` works for anything holding a reference to the class, and `error.name` is a stable thing for a dApp to branch on. A property bag on an `Error` would give the same `code` with none of that, for no saving. Two deliberate limits on the shape: - **Pass-through, not a whitelist.** Whatever `code` arrived is carried verbatim rather than matched against a list of known values. A code the provider has never heard of is still the truth about what happened, and a code added to the background later must reach the page without editing this file. Covered by a test that feeds it 4900, which nothing emits today. - **A code-less error stays a plain `Error`.** Several background paths report `{message}` with no code. Those keep exactly today's behaviour, with no `code` property at all — not `code: undefined`. A `ProviderRpcError` whose `code` is undefined would advertise a conformance it does not have, and `'code' in err` is exactly what a careful dApp asks. Asserted both ways. `message` is untouched in every case, including the `"Request failed"` fallback for an error that arrives without one. Nothing the background produces changes. ## Codes the background emits, and which now reach the page Read out of `src/background/index.js`; every one of them now arrives on the page's `Error`, because the provider passes them through rather than enumerating them. | Code | Where it is produced | Reaches the page | | --- | --- | --- | | `4001` | site connection denied or rejected at the prompt; tx and sign approvals rejected, closed by the window manager, or torn down on disconnect (7 sites) | yes | | `4100` | `personal_sign` / `eth_sign` / `eth_signTypedData_v4` from a site that is not connected, or naming an address that is not the active one (6 sites) | yes | | `4902` | `wallet_switchEthereumChain` / `wallet_addEthereumChain` for a chain that is not Mainnet or Sepolia (2 sites) | yes | | none | `"No accounts available"`, `"Unsupported method: …"`, a failed proxy RPC, a failed tx population, an address that changed mid-preparation | unchanged: plain `Error`, same message | `4200`, `4900` and `4901` are named by EIP-1193 but are not produced anywhere in this codebase today, so nothing is invented for them; the pass-through means they need no change here if they ever are. The code-less row is untouched on purpose — this unit is confined to what the provider surfaces, not to what the background produces. That `"Unsupported method"` arguably wants `4200` is filed separately as https://git.eeqj.de/sneak/AutistMask/issues/279. ## Every request path `request`, `enable`, `send(method, params)`, `send({method, params})` and `sendAsync` all funnel through the single `AUTISTMASK_RESPONSE` listener, so one conversion point covers all of them. `tests/inpageErrors.test.js` drives each entry point separately and asserts the code on each, so the claim is tested rather than argued. ## Tests `tests/inpageErrors.test.js` (new, 17 cases) loads the real `src/content/inpage.js` — a bare IIFE, not a module — by compiling it with its globals as function parameters, so nothing leaks between tests and the source compiles in the test's own realm, which is what makes `expect(err).toBeInstanceOf(Error)` meaningful. There is no jsdom in this repo; this follows the hand-rolled stub convention of `tests/txStatus.test.js`. Not vacuous — with the `inpage.js` change stashed and the test file unchanged, 12 of the 17 fail: ``` ✕ a user rejection arrives as code 4001 ✕ it is a ProviderRpcError, and an Error ✕ 4100 unauthorized arrives intact ✕ 4902 unrecognized chain arrives intact ✕ a code the provider has never heard of is passed through ✕ data is carried when the boundary sent it ✓ no data property is invented when the boundary sent none ✓ a coded error keeps the message byte for byte ✓ an error the background sent with no code keeps its message ✓ an error with no code gets no code property at all ✕ an error with no message keeps the generic fallback ✕ request() ✕ enable() ✕ send(method, params) ✕ send({ method, params }) ✕ sendAsync() hands the code to its callback ✓ a result still resolves Tests: 12 failed, 5 passed, 17 total ``` The five that pass unchanged are the ones asserting behaviour this deliberately preserves: the untouched messages, and the absence of `code`/`data` where the boundary sent none. That is the point of having them. ### The e2e probe, flipped https://git.eeqj.de/sneak/AutistMask/pulls/273 landed four rejection cases that printed `page Error.code=undefined page Error carries a code=false` rather than asserting, so this unit could flip them on. `assertUserRejection()` in `tests/e2e/run.js` now requires the code on the page's `Error` as well as on the wire, and requires the `ProviderRpcError` name; the fixture in `tests/e2e/network.js` records `name` alongside the `code` and `hasCode` it already captured. Failing before the provider change, with the flipped assertion in place — `make test-e2e`, exit 1: ``` not ok 29 - eth_requestAccounts rejected at the prompt returns a rejection (#183) eth_requestAccounts rejection reached the page as an error with no code property at all, so a dApp cannot tell the user's refusal from a failure: {"settled":"rejected","message":"User rejected the request.","name":"Error","hasCode":false} not ok 32 - personal_sign rejected returns a rejection to the page (#183) personal_sign rejection reached the page as an error with no code property at all, so a dApp cannot tell the user's refusal from a failure: {"settled":"rejected","message":"User rejected the request.","name":"Error","hasCode":false} not ok 34 - eth_signTypedData_v4 rejected returns a rejection to the page (#183) eth_signTypedData_v4 rejection reached the page as an error with no code property at all, so a dApp cannot tell the user's refusal from a failure: {"settled":"rejected","message":"User rejected the request.","name":"Error","hasCode":false} not ok 36 - eth_sendTransaction rejected broadcasts nothing (#183) eth_sendTransaction rejection reached the page as an error with no code property at all, so a dApp cannot tell the user's refusal from a failure: {"settled":"rejected","message":"User rejected the request.","name":"Error","hasCode":false} # 33/37 tests passed # FAILED ``` Note the recorded `"message":"User rejected the request."` in the failing output above and in the passing run below: byte-identical, which is the `message`-unchanged claim shown rather than asserted in prose. Passing after, on the rebased branch — `make test-e2e`, exit 0: ``` # eth_requestAccounts rejection: code 4001 on the wire and on the page's ProviderRpcError # personal_sign rejection: code 4001 on the wire and on the page's ProviderRpcError # eth_signTypedData_v4 rejection: code 4001 on the wire and on the page's ProviderRpcError # eth_sendTransaction rejection: code 4001 on the wire and on the page's ProviderRpcError # 37/37 tests passed ``` ## Verification Both re-run after rebasing onto `next` at `c755a5e`. - `make check` — exit 0. 681 tests in 28 suites, `test-verify-build: 18 case(s) passed`, prettier clean. - `make test-e2e` — exit 0, 37/37, in the pinned Playwright container. ## Scope `src/content/inpage.js` and the tests only. `src/content/index.js`, `src/background/index.js` and `src/popup/views/approval.js` are untouched, so this does not collide with the work on https://git.eeqj.de/sneak/AutistMask/issues/153. The `window.close()` accommodation the harness makes for https://git.eeqj.de/sneak/AutistMask/issues/275 is left exactly as it was.
clawbot added 1 commit 2026-08-12 13:36:49 +02:00
fix: carry EIP-1193 error codes through to the page (closes #274)
All checks were successful
check / check (push) Successful in 30s
Details
9317d4386e 
src/content/inpage.js rebuilt every failure as `new Error(error.message)`,
so the `code` the background produced and the content script relayed intact
was dropped in the last hop. A dApp checking `err.code === 4001` — the
standard way to tell "the user said no" from "something broke" — saw
`undefined`, which makes a wallet the user deliberately declined
indistinguishable from a wallet that failed.

The provider now rejects with an EIP-1193 `ProviderRpcError` carrying the
`code` and, where the boundary sent one, `data`. A class rather than
properties attached to an `Error`: the object crosses no boundary after
construction — it is built in the page's own realm and handed straight to
the caller's catch — so the prototype survives and `name` is a stable thing
for a dApp to see.

Whatever code arrived is passed through verbatim rather than matched
against a list. The background emits 4001, 4100 and 4902 on the RPC path
today; a code added later must reach the page without editing the provider.
An error the background sent with no code stays a plain `Error` with no
`code` property at all — a `ProviderRpcError` whose `code` is `undefined`
would advertise a conformance it does not have. `message` is unchanged in
every case, and nothing the background produces changes.

Every entry point the provider exposes funnels through the one response
listener, so `request`, `enable`, `send` and `sendAsync` are all covered;
tests/inpageErrors.test.js loads the real inpage.js against a stub window
and asserts each of them, both codes and the untouched messages.

The e2e probe added for #183, which printed the missing code on all four
rejected flows rather than asserting it, now requires code 4001 on the
page's Error as well as on the wire. Without the provider change it fails
on all four with "reached the page as an error with no code property at
all".
clawbot self-assigned this 2026-08-12 13:38:01 +02:00
clawbot added the needs-review label 2026-08-12 13:38:17 +02:00
clawbot commented 2026-08-12 13:46:50 +02:00
Author
Collaborator
Copy Link

PASS — independently verified: make check 28 suites / 681 tests / test-verify-build 18 cases / prettier clean (exit 0), make test-e2e 37/37 (exit 0), reverting the one production line reproduces 12/17 unit failures and e2e 33/37 on cases 29/32/34/36, CI green on 9317d43, fast-forwardable onto next at c755a5e, scope confined to src/content/inpage.js plus tests.

Anomalies and disclosures, none of them blocking:

  • Message byte-identity was proven, not sampled. A differential probe ran the pre-PR and post-PR provider side by side over 27 boundary payloads — every page-facing error object src/background/index.js can produce, plus hostile shapes (code as a string, null, 0, negative, MAX_SAFE_INTEGER, an object, an array; message absent, empty, null, a number; error as a bare string, number, boolean, array). All 27 messages identical. This was a standalone node script against the shipped file, not a jest run.
  • No error path still loses its code. Independent grep: 4001 at 6 sites (319, 364, 701, 843, 939, 1068), 4100 at 6, 4902 at 2, and 5 distinct code-less messages across 8 return sites. The PR body says "7 sites" for 4001 and "5 sites" for the code-less class — the first is one over, the second counts message classes rather than return sites. Prose only; the pass-through carries whatever arrives, so the miscount changes nothing.
  • The realm claim was checked in the browser, not read. The e2e dApp fixture was temporarily instrumented to record error instanceof Error, the prototype chain, and typeof error.stack; all four rejected flows returned isError:true, ProviderRpcError.prototype -> Error.prototype, stack a string, with "message":"User rejected the request." byte-identical to the pre-fix run. Instrumentation reverted, tree clean at 9317d43.
  • Hostile codes behave sanely. A non-numeric code passes through verbatim and err.code === 4001 is simply false — nothing throws, nothing coerces.
  • data without code is dropped, since such an error falls to the plain-Error branch. Unreachable today: no background path emits data at all. Noted, not a defect.
  • Out of scope, pre-existing: src/background/index.js:868 calls handleRpc(...).then(...) with no .catch(), so a throw inside it never calls sendResponse and the page's promise hangs. Untouched by this PR.
PASS — independently verified: `make check` 28 suites / 681 tests / `test-verify-build` 18 cases / prettier clean (exit 0), `make test-e2e` 37/37 (exit 0), reverting the one production line reproduces 12/17 unit failures and e2e 33/37 on cases 29/32/34/36, CI green on `9317d43`, fast-forwardable onto `next` at `c755a5e`, scope confined to `src/content/inpage.js` plus tests. Anomalies and disclosures, none of them blocking: - **Message byte-identity was proven, not sampled.** A differential probe ran the pre-PR and post-PR provider side by side over 27 boundary payloads — every page-facing error object `src/background/index.js` can produce, plus hostile shapes (`code` as a string, `null`, `0`, negative, `MAX_SAFE_INTEGER`, an object, an array; `message` absent, empty, `null`, a number; `error` as a bare string, number, boolean, array). All 27 messages identical. This was a standalone node script against the shipped file, not a jest run. - **No error path still loses its code.** Independent grep: `4001` at 6 sites (319, 364, 701, 843, 939, 1068), `4100` at 6, `4902` at 2, and 5 distinct code-less messages across 8 return sites. The PR body says "7 sites" for `4001` and "5 sites" for the code-less class — the first is one over, the second counts message classes rather than return sites. Prose only; the pass-through carries whatever arrives, so the miscount changes nothing. - **The realm claim was checked in the browser, not read.** The e2e dApp fixture was temporarily instrumented to record `error instanceof Error`, the prototype chain, and `typeof error.stack`; all four rejected flows returned `isError:true`, `ProviderRpcError.prototype -> Error.prototype`, `stack` a string, with `"message":"User rejected the request."` byte-identical to the pre-fix run. Instrumentation reverted, tree clean at `9317d43`. - **Hostile codes behave sanely.** A non-numeric `code` passes through verbatim and `err.code === 4001` is simply `false` — nothing throws, nothing coerces. - **`data` without `code` is dropped**, since such an error falls to the plain-`Error` branch. Unreachable today: no background path emits `data` at all. Noted, not a defect. - **Out of scope, pre-existing:** `src/background/index.js:868` calls `handleRpc(...).then(...)` with no `.catch()`, so a throw inside it never calls `sendResponse` and the page's promise hangs. Untouched by this PR.
clawbot merged commit 9dcd875dd4 into next 2026-08-12 13:47:34 +02:00
clawbot deleted branch fix/issue-274-eip1193-error-codes 2026-08-12 13:47:34 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label needs-review
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees clawbot
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#278
Delete Branch "fix/issue-274-eip1193-error-codes"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?