fix: answer the page when a background handler throws (closes #280)
All checks were successful
check / check (push) Successful in 46s
All checks were successful
check / check (push) Successful in 46s
handleRpc(...).then(sendResponse) had no .catch(), and sendResponse is the only
thing that settles the dApp's window.ethereum.request() promise. Any throw
inside handleRpc therefore sent nothing back: the content script posted nothing,
and the page's promise stayed pending forever with no error and no timeout,
indistinguishable from a slow wallet. handleRpc does real work -- state loads,
provider calls, transaction population, approval plumbing -- so "it does not
throw today" was not a property anyone was maintaining.
A rejected handleRpc now answers { code: -32603, message }. -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 none was
invented for it. The cause is not put in the message: the page gets a stable
sentence, the background console gets the method and the throw, so the failure
is visible rather than swallowed.
The two async IIFEs behind AUTISTMASK_TX_RESPONSE and AUTISTMASK_SIGN_RESPONSE
are the same shape one level down. Every statement 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 answered. Each gets a last-resort .catch() that settles
the approval through settleApproval() -- the existing chokepoint, with no new
delete or resolve -- and answers the popup. The transaction one reports the
broadcast stage, because it cannot tell whether the transaction reached the
network and that is the wording that does not invite a second send. Every other
message handler on the path is synchronous and cannot leave a promise pending.
Each of the three is driven by a real failure rather than a hook in the handler:
a rejecting extension-storage read, which getState() awaits unguarded, and a
failure classifier that throws while classifying a genuine verification failure.
All three were demonstrated failing against the unfixed code, the RPC one with
sendResponse at zero calls, which is precisely the page-side hang.
This commit is contained in:
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user