TODO.md

@@ -45,6 +45,25 @@ undefined identifiers, which is how
 
 # Completed Steps
 
+- 2026-08-14: A background message handler that throws now rejects the page
+  instead of hanging it. `handleRpc(...).then(sendResponse)` had no `.catch()`,
+  and `sendResponse` is the only thing that settles the dApp's
+  `window.ethereum.request()` promise — so any throw inside `handleRpc` left
+  that promise pending forever, with no error and no timeout, indistinguishable
+  from a slow wallet. It now answers `{ code: -32603, message }` (the JSON-RPC
+  internal error EIP-1474 defines and EIP-1193 defers to; no EIP-1193 4xxx code
+  describes "the wallet broke" and none was invented) and logs the method and
+  the throw to the background console rather than swallowing them. The two async
+  IIFEs behind `AUTISTMASK_TX_RESPONSE` and `AUTISTMASK_SIGN_RESPONSE` were the
+  same shape one level down — every statement inside a `try`, but a throw out of
+  a `catch` block escaping unhandled — and each got a last-resort `.catch()`
+  settling the approval through `settleApproval()` and answering the popup; the
+  transaction one reports the broadcast stage, because it cannot tell whether
+  the transaction reached the network. Every other handler on the path is
+  synchronous. All three are driven by real failures — a rejecting storage read,
+  and a failure classifier that throws while classifying a genuine verification
+  failure — and were demonstrated failing first, the RPC one with `sendResponse`
+  at zero calls ([#280](https://git.eeqj.de/sneak/AutistMask/issues/280)).
 - 2026-08-12: EIP-1193 error codes now reach the page. `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

src/background/index.js

@@ -57,6 +57,16 @@ const connectedSites = {};
 // Pending approval requests: { id: { origin, hostname, resolve } }
 const pendingApprovals = {};
 
+// What the page is told when a request failed in a way the wallet has no
+// specific answer for. -32603 is the JSON-RPC internal error EIP-1474 defines
+// and EIP-1193 defers to for RPC-layer failures; no EIP-1193 4xxx code
+// describes "the wallet broke", and one is not invented here. The cause is
+// logged rather than put in the message: the page gets a stable sentence, the
+// background console gets the throw.
+const INTERNAL_ERROR_CODE = -32603;
+const INTERNAL_ERROR_MESSAGE =
+    "AutistMask could not complete this request because of an internal error.";
+
 async function getState() {
     const result = await storageApi.get("autistmask");
     return (
@@ -865,8 +875,25 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 // keep fallback
             }
         }
-        handleRpc(msg.method, msg.params, trustedOrigin).then((response) => {
+        handleRpc(msg.method, msg.params, trustedOrigin)
+            .then((response) => {
                 sendResponse(response);
+            })
+            .catch((err) => {
+                // Without this the page's window.ethereum.request() promise
+                // stays pending forever: no response is sent, the content
+                // script posts nothing back, and the dApp cannot tell the
+                // failure from a slow wallet. handleRpc does real work —
+                // state loads, provider calls, transaction population — so
+                // "it does not throw today" is not a property anyone is
+                // maintaining.
+                log.errorf("RPC request failed:", msg.method, err);
+                sendResponse({
+                    error: {
+                        code: INTERNAL_ERROR_CODE,
+                        message: INTERNAL_ERROR_MESSAGE,
+                    },
+                });
             });
         return true;
     }
@@ -1051,7 +1078,31 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                     stage: TX_STAGE_BROADCAST,
                 });
             }
-        })();
+        })().catch((e) => {
+            // Every statement above is inside a try, but a throw from one of
+            // the catch blocks escapes as an unhandled rejection and neither
+            // the popup nor the page is ever answered. Settle both, through
+            // the same chokepoint as every other retirement. The stage is
+            // broadcast because this cannot tell whether the transaction
+            // reached the network, and that is the wording that does not
+            // invite a second send.
+            log.errorf("transaction approval response failed:", e);
+            settleApproval(
+                msg.id,
+                {
+                    error: {
+                        code: INTERNAL_ERROR_CODE,
+                        message: INTERNAL_ERROR_MESSAGE,
+                    },
+                },
+                { holdsClaim: true },
+            );
+            sendResponse({
+                error: INTERNAL_ERROR_MESSAGE,
+                retryable: false,
+                stage: TX_STAGE_BROADCAST,
+            });
+        });
         return true;
     }
 
@@ -1135,7 +1186,25 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 }
                 sendResponse({ error: errMsg, retryable });
             }
-        })();
+        })().catch((e) => {
+            // Same shape as the transaction path: a throw out of the catch
+            // block above would leave the popup and the page both waiting.
+            log.errorf("sign approval response failed:", e);
+            settleApproval(
+                msg.id,
+                {
+                    error: {
+                        code: INTERNAL_ERROR_CODE,
+                        message: INTERNAL_ERROR_MESSAGE,
+                    },
+                },
+                { holdsClaim: true },
+            );
+            sendResponse({
+                error: INTERNAL_ERROR_MESSAGE,
+                retryable: false,
+            });
+        });
         return true;
     }
 

tests/backgroundApproval.test.js

@@ -133,6 +133,14 @@ function loadBackground(options) {
         ensureRecurringAlarms: jest.fn(async () => {}),
         registerAlarmHandlers: jest.fn(),
     }));
+    // The real verification module, except where a test replaces one export
+    // with a throw to drive the handler's own error handling into failing.
+    if (opts.approvalVerify) {
+        jest.doMock("../src/shared/approvalVerify", () => ({
+            ...jest.requireActual("../src/shared/approvalVerify"),
+            ...opts.approvalVerify,
+        }));
+    }
 
     const persisted = {
         wallets: [
@@ -152,7 +160,10 @@ function loadBackground(options) {
     global.chrome = {
         storage: {
             local: {
-                get: jest.fn(async () => ({ autistmask: persisted })),
+                get: jest.fn(
+                    opts.storageGet ||
+                        (async () => ({ autistmask: persisted })),
+                ),
                 set: jest.fn(async () => {}),
             },
         },
@@ -280,6 +291,25 @@ async function settle() {
     for (let i = 0; i < 50; i++) await Promise.resolve();
 }
 
+// Node aborts the worker process on an unhandled rejection; an extension
+// service worker does not — the promise is simply never settled, nothing is
+// sent back, and the page's window.ethereum.request() waits forever. Recording
+// them instead of dying on them keeps that difference visible: the assertion
+// that the page WAS answered is what reports the failure, and the recording is
+// asserted empty alongside it.
+const unhandledRejections = [];
+process.on("unhandledRejection", (reason) => {
+    unhandledRejections.push(reason);
+});
+
+// Node reports an unhandled rejection on the macrotask turn after the promise
+// was left unhandled, which is past everything settle() waits for.
+async function settleIncludingRejections() {
+    await settle();
+    await new Promise((resolve) => setImmediate(resolve));
+    await new Promise((resolve) => setImmediate(resolve));
+}
+
 afterEach(() => {
     delete global.chrome;
     jest.resetModules();
@@ -1034,6 +1064,147 @@ describe("a claimed approval outlives every other retirement path", () => {
     });
 });
 
+// A handler that throws must still answer. `sendResponse` is the only thing
+// that settles the page's window.ethereum.request() promise, so a throw that
+// escapes a handler leaves that promise pending forever — no error, no
+// timeout, indistinguishable from a slow wallet. Each case below drives a real
+// throw out of a handler rather than asserting the catch block exists.
+describe("a handler that throws still settles the page", () => {
+    const INTERNAL_ERROR = {
+        code: -32603,
+        message:
+            "AutistMask could not complete this request because of an internal error.",
+    };
+
+    let errorLog;
+    beforeEach(() => {
+        errorLog = jest.spyOn(console, "error").mockImplementation(() => {});
+        unhandledRejections.length = 0;
+    });
+    afterEach(() => {
+        errorLog.mockRestore();
+    });
+
+    // getState() awaits extension storage unguarded, and every read path in
+    // handleRpc goes through it. A storage read that rejects is the whole
+    // failure — no hook in the handler itself.
+    test("a rejected handleRpc rejects the page instead of hanging it", async () => {
+        const bg = loadBackground({
+            storageGet: async () => {
+                throw new Error("storage unavailable");
+            },
+        });
+
+        const answer = bg.send(
+            { type: "AUTISTMASK_RPC", method: "eth_accounts", params: [] },
+            { origin: ORIGIN },
+        );
+        await settleIncludingRejections();
+
+        // The channel is held open for the async answer, and the answer
+        // arrives.
+        expect(answer.kept).toBe(true);
+        expect(answer.sendResponse).toHaveBeenCalledWith({
+            error: INTERNAL_ERROR,
+        });
+        expect(unhandledRejections).toEqual([]);
+        // Not swallowed: the throw is on the background console, which is how
+        // this class gets caught in future.
+        expect(errorLog).toHaveBeenCalledWith(
+            "[AutistMask]",
+            "RPC request failed:",
+            "eth_accounts",
+            expect.objectContaining({ message: "storage unavailable" }),
+        );
+    });
+
+    // The transaction response handler wraps every statement in a try, so what
+    // escapes it is a throw from inside one of its catch blocks. Here the
+    // failure classifier itself throws while classifying a real verification
+    // failure — the approval is left claimed, so nothing else can settle it.
+    test("a throw while handling a failed transaction settles both the page and the popup", async () => {
+        const bg = loadBackground({
+            approvalVerify: {
+                describeTxFailure: () => {
+                    throw new Error("classifier broke");
+                },
+            },
+        });
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        // A real verification failure: the artifact is signed at a nonce the
+        // approval never displayed.
+        const answer = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(NONCE + 1),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settleIncludingRejections();
+
+        expect(bg.broadcastTransaction).not.toHaveBeenCalled();
+        expect(pending.result()).toEqual({ error: INTERNAL_ERROR });
+        expect(answer.sendResponse).toHaveBeenCalledWith({
+            error: INTERNAL_ERROR.message,
+            retryable: false,
+            // The handler cannot tell whether the transaction reached the
+            // network, so the popup must not say "start again from the site".
+            stage: "broadcast",
+        });
+        expect(unhandledRejections).toEqual([]);
+        expect(errorLog).toHaveBeenCalledWith(
+            "[AutistMask]",
+            "transaction approval response failed:",
+            expect.objectContaining({ message: "classifier broke" }),
+        );
+    });
+
+    test("a throw while handling a failed signature settles both the page and the popup", async () => {
+        const bg = loadBackground({
+            approvalVerify: {
+                failureIsRetryable: () => {
+                    throw new Error("classifier broke");
+                },
+            },
+        });
+        const pending = bg.requestSign();
+        await settle();
+
+        // A real verification failure: the active address moved after the
+        // approval was raised.
+        bg.setActiveAddress(other.address);
+        const answer = bg.send(
+            {
+                type: "AUTISTMASK_SIGN_RESPONSE",
+                id: pending.id(),
+                approved: true,
+                signature: await signer.signMessage(
+                    Buffer.from(MESSAGE.slice(2), "hex"),
+                ),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settleIncludingRejections();
+
+        expect(pending.result()).toEqual({ error: INTERNAL_ERROR });
+        expect(answer.sendResponse).toHaveBeenCalledWith({
+            error: INTERNAL_ERROR.message,
+            retryable: false,
+        });
+        expect(unhandledRejections).toEqual([]);
+        expect(errorLog).toHaveBeenCalledWith(
+            "[AutistMask]",
+            "sign approval response failed:",
+            expect.objectContaining({ message: "classifier broke" }),
+        );
+    });
+});
+
 describe("popup-only messages", () => {
     test("a page sender cannot answer an approval", async () => {
         const bg = loadBackground();