Compare commits
1 Commits
issue-188-
...
issue-280-
| Author | SHA1 | Date | |
|---|---|---|---|
| 9665ac448e |
19
TODO.md
19
TODO.md
@@ -45,6 +45,25 @@ undefined identifiers, which is how
|
|||||||
|
|
||||||
# Completed Steps
|
# 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`
|
- 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
|
rebuilt every failure as `new Error(error.message)`, so the code the
|
||||||
background produced and the content script relayed intact was dropped in the
|
background produced and the content script relayed intact was dropped in the
|
||||||
|
|||||||
@@ -57,6 +57,16 @@ const connectedSites = {};
|
|||||||
// Pending approval requests: { id: { origin, hostname, resolve } }
|
// Pending approval requests: { id: { origin, hostname, resolve } }
|
||||||
const pendingApprovals = {};
|
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() {
|
async function getState() {
|
||||||
const result = await storageApi.get("autistmask");
|
const result = await storageApi.get("autistmask");
|
||||||
return (
|
return (
|
||||||
@@ -865,9 +875,26 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
|||||||
// keep fallback
|
// keep fallback
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
handleRpc(msg.method, msg.params, trustedOrigin).then((response) => {
|
handleRpc(msg.method, msg.params, trustedOrigin)
|
||||||
sendResponse(response);
|
.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;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1051,7 +1078,31 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
|||||||
stage: TX_STAGE_BROADCAST,
|
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;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1135,7 +1186,25 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
|||||||
}
|
}
|
||||||
sendResponse({ error: errMsg, retryable });
|
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;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -133,6 +133,14 @@ function loadBackground(options) {
|
|||||||
ensureRecurringAlarms: jest.fn(async () => {}),
|
ensureRecurringAlarms: jest.fn(async () => {}),
|
||||||
registerAlarmHandlers: jest.fn(),
|
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 = {
|
const persisted = {
|
||||||
wallets: [
|
wallets: [
|
||||||
@@ -152,7 +160,10 @@ function loadBackground(options) {
|
|||||||
global.chrome = {
|
global.chrome = {
|
||||||
storage: {
|
storage: {
|
||||||
local: {
|
local: {
|
||||||
get: jest.fn(async () => ({ autistmask: persisted })),
|
get: jest.fn(
|
||||||
|
opts.storageGet ||
|
||||||
|
(async () => ({ autistmask: persisted })),
|
||||||
|
),
|
||||||
set: jest.fn(async () => {}),
|
set: jest.fn(async () => {}),
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@@ -280,6 +291,25 @@ async function settle() {
|
|||||||
for (let i = 0; i < 50; i++) await Promise.resolve();
|
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(() => {
|
afterEach(() => {
|
||||||
delete global.chrome;
|
delete global.chrome;
|
||||||
jest.resetModules();
|
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", () => {
|
describe("popup-only messages", () => {
|
||||||
test("a page sender cannot answer an approval", async () => {
|
test("a page sender cannot answer an approval", async () => {
|
||||||
const bg = loadBackground();
|
const bg = loadBackground();
|
||||||
|
|||||||
Reference in New Issue
Block a user