From 5f332f40745cabef82c179ae4e1f2831d2e9d7fd Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 6 Oct 2026 07:59:25 +0000 Subject: [PATCH] fix: a popup reload no longer logs the requests it cancels, except on the transaction detail and confirmation screens (closes #475) The transaction lists and ENS name lookups on the address and token screens, the address scan after a wallet is created, the endpoint checks in Settings, the wait screen's receipt check, the Send screen's Max fee estimate and the token lookup on both add-token screens now check the signal the popup aborts on pagehide before reporting a failed request. scanForAddresses(), resolveEnsNames() and lookupTokenInfo() take the signal. End-to-end tests reload the popup on the address screen and during the address scan with their requests held. Jest tests show each of these reports a real failure and stays silent once the popup has closed. The transaction detail and confirmation screens are left out: they discard the popup context that carries the signal. Model: opus-5-5 --- TODO.md | 16 +++ src/popup/views/addToken.js | 4 + src/popup/views/addWallet.js | 14 +- src/popup/views/addressDetail.js | 4 + src/popup/views/addressToken.js | 4 + src/popup/views/send.js | 3 + src/popup/views/settings.js | 4 + src/popup/views/settingsAddToken.js | 4 + src/popup/views/txStatus.js | 3 + src/shared/balances.js | 36 +++-- src/shared/ens.js | 22 +-- tests/addressScanCancelled.test.js | 121 +++++++++++++++++ tests/balanceRefreshCancelled.test.js | 40 +++++- tests/e2e/network.js | 17 ++- tests/e2e/run.js | 55 ++++++-- tests/flashLine.test.js | 1 + tests/sendMax.test.js | 53 +++++++- tests/settingsEndpointCheck.test.js | 25 +++- tests/tokenLookupCancelled.test.js | 167 +++++++++++++++++++++++ tests/transactionListCancelled.test.js | 181 +++++++++++++++++++++++++ tests/txStatus.test.js | 37 ++++- 21 files changed, 762 insertions(+), 49 deletions(-) create mode 100644 tests/addressScanCancelled.test.js create mode 100644 tests/tokenLookupCancelled.test.js create mode 100644 tests/transactionListCancelled.test.js diff --git a/TODO.md b/TODO.md index be03229..aadc40d 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,22 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-06: Reloading or closing the popup no longer logs a request it cancels + as a failure in the transaction lists and ENS name lookups on the address and + token screens, the address scan after a wallet is created, the endpoint checks + in Settings, the wait screen's receipt check, the Send screen's Max fee + estimate, or the token lookup on the two add-token screens + ([#475](https://git.eeqj.de/sneak/AutistMask/issues/475)). Each checks the + signal the popup aborts on `pagehide` + ([#218](https://git.eeqj.de/sneak/AutistMask/issues/218)) before reporting a + failure; a real failure is still logged. `scanForAddresses()`, + `resolveEnsNames()` and `lookupTokenInfo()` take the signal. The e2e suite + reloads the popup on the address screen with Blockscout held, and during the + address scan with that scan held; jest tests cover each of these with the + popup open and closed. Left out: the transaction detail screen and the + confirmation screen (its fee estimate and its recipient checks), because they + discard the popup context that carries the signal. + - 2026-10-06: The canonical files are re-vendored from `sneak/prompts` at `dd4027b` ([#472](https://git.eeqj.de/sneak/AutistMask/issues/472)). The `Dockerfile` has separate `lint` and `test` phases, and its last stage depends diff --git a/src/popup/views/addToken.js b/src/popup/views/addToken.js index a2114bb..45829e3 100644 --- a/src/popup/views/addToken.js +++ b/src/popup/views/addToken.js @@ -51,6 +51,7 @@ function init(ctx) { contractAddr, state.rpcUrl, state.networkId, + ctx.pageClosed, ); log.infof("Adding token", info.symbol, contractAddr); state.trackedTokens.push({ @@ -68,6 +69,9 @@ function init(ctx) { } require("./addressDetail").show(); } catch (e) { + // Cancelled by the popup closing, not failed: see pageClosed in + // src/popup/index.js. + if (ctx.pageClosed.aborted) return; const detail = e.shortMessage || e.message || String(e); log.errorf("Adding token failed for", contractAddr, detail); // lookupTokenInfo() rejects a contract with a one-line message diff --git a/src/popup/views/addWallet.js b/src/popup/views/addWallet.js index c9b7087..97f7f9a 100644 --- a/src/popup/views/addWallet.js +++ b/src/popup/views/addWallet.js @@ -190,7 +190,12 @@ async function importMnemonic(ctx) { // Scan for used HD addresses beyond index 0. showFlash("Scanning for addresses...", 30000); - const scan = await scanForAddresses(xpub, state.rpcUrl, state.networkId); + const scan = await scanForAddresses( + xpub, + state.rpcUrl, + state.networkId, + ctx.pageClosed, + ); if (scan.addresses.length > 1) { wallet.addresses = scan.addresses.map((a) => ({ address: a.address, @@ -300,7 +305,12 @@ async function importXprvKey(ctx) { // Scan for used HD addresses beyond index 0. showFlash("Scanning for addresses...", 30000); - const scan = await scanForAddresses(xpub, state.rpcUrl, state.networkId); + const scan = await scanForAddresses( + xpub, + state.rpcUrl, + state.networkId, + ctx.pageClosed, + ); if (scan.addresses.length > 1) { wallet.addresses = scan.addresses.map((a) => ({ address: a.address, diff --git a/src/popup/views/addressDetail.js b/src/popup/views/addressDetail.js index f100536..001e4ec 100644 --- a/src/popup/views/addressDetail.js +++ b/src/popup/views/addressDetail.js @@ -135,6 +135,7 @@ async function loadTransactions(address) { counterparties, state.rpcUrl, state.networkId, + ctx.pageClosed, ); } catch { ensNameMap = new Map(); @@ -143,6 +144,9 @@ async function loadTransactions(address) { renderTransactions(txs); } catch (e) { + // Cancelled by the popup closing, not failed: see pageClosed in + // src/popup/index.js. + if (ctx.pageClosed.aborted) return; log.errorf("loadTransactions failed:", e.message); $("tx-list").innerHTML = '
Failed to load transactions.
'; diff --git a/src/popup/views/addressToken.js b/src/popup/views/addressToken.js index a544789..207d0fb 100644 --- a/src/popup/views/addressToken.js +++ b/src/popup/views/addressToken.js @@ -219,6 +219,7 @@ async function loadTransactions(address, tokenId) { counterparties, state.rpcUrl, state.networkId, + ctx.pageClosed, ); } catch { ensNameMap = new Map(); @@ -227,6 +228,9 @@ async function loadTransactions(address, tokenId) { renderTransactions(txs); } catch (e) { + // Cancelled by the popup closing, not failed: see pageClosed in + // src/popup/index.js. + if (ctx.pageClosed.aborted) return; log.errorf("loadTransactions failed:", e.message); $("address-token-tx-list").innerHTML = '
Failed to load transactions.
'; diff --git a/src/popup/views/send.js b/src/popup/views/send.js index 8e031a8..fe738bb 100644 --- a/src/popup/views/send.js +++ b/src/popup/views/send.js @@ -292,6 +292,9 @@ async function fillMaxAmount() { ]); feeWei = feeReserveWei(gasLimit, feeData); } catch (e) { + // Cancelled by the popup closing, not failed: see pageClosed in + // src/popup/index.js. + if (ctx.pageClosed.aborted) return; log.errorf( "max amount fee estimate failed:", e.shortMessage || e.message, diff --git a/src/popup/views/settings.js b/src/popup/views/settings.js index cd79548..4661fd4 100644 --- a/src/popup/views/settings.js +++ b/src/popup/views/settings.js @@ -273,6 +273,9 @@ function init(ctx) { return; } } catch { + // Cancelled by the popup closing, not failed: see pageClosed in + // src/popup/index.js. + if (ctx.pageClosed.aborted) return; // Not the error's message: fetch puts the whole URL, password and // key included, in the message of the error it throws for a URL // with a user name and password or one it cannot parse. @@ -299,6 +302,7 @@ function init(ctx) { return; } } catch { + if (ctx.pageClosed.aborted) return; // Not the error's message, as for the RPC check above. log.errorf("Blockscout validation failed:", urlOrigin(url)); showFlash("Could not reach endpoint."); diff --git a/src/popup/views/settingsAddToken.js b/src/popup/views/settingsAddToken.js index 2789e8f..1ad1508 100644 --- a/src/popup/views/settingsAddToken.js +++ b/src/popup/views/settingsAddToken.js @@ -135,6 +135,7 @@ function init(_ctx) { addr, state.rpcUrl, state.networkId, + ctx.pageClosed, ); log.infof("Adding token", info.symbol, addr); state.trackedTokens.push({ @@ -152,6 +153,9 @@ function init(_ctx) { renderDropdown(); ctx.doRefreshAndRender(); } catch (e) { + // Cancelled by the popup closing, not failed: see pageClosed in + // src/popup/index.js. + if (ctx.pageClosed.aborted) return; const detail = e.shortMessage || e.message || String(e); log.errorf("Adding token failed for", addr, detail); // lookupTokenInfo() rejects a contract with a one-line message diff --git a/src/popup/views/txStatus.js b/src/popup/views/txStatus.js index cc65a87..78530e5 100644 --- a/src/popup/views/txStatus.js +++ b/src/popup/views/txStatus.js @@ -132,6 +132,9 @@ function startWait(txInfo, txHash, broadcastTime, pollNow) { try { receipt = await provider.getTransactionReceipt(txHash); } catch (e) { + // Cancelled by the popup closing, not failed: see pageClosed in + // src/popup/index.js. + if (ctx.pageClosed.aborted) return; // A thrown lookup means "no answer this tick", not "no // receipt": the RPC failed, the chain said nothing. Declaring // the timeout off it would report a confirmed transaction as diff --git a/src/shared/balances.js b/src/shared/balances.js index eabbed2..6b294ba 100644 --- a/src/shared/balances.js +++ b/src/shared/balances.js @@ -294,7 +294,8 @@ async function refreshBalances( // Look up token metadata from its contract. // Calls symbol() and decimals() to verify it implements ERC-20. -async function lookupTokenInfo(contractAddress, rpcUrl, networkId) { +// `signal` is as for fetchTokenBalances(). +async function lookupTokenInfo(contractAddress, rpcUrl, networkId, signal) { log.debugf("lookupTokenInfo", contractAddress, "rpc:", urlOrigin(rpcUrl)); const provider = getProvider(rpcUrl, networkId); const contract = new Contract(contractAddress, ERC20_ABI, provider); @@ -304,7 +305,9 @@ async function lookupTokenInfo(contractAddress, rpcUrl, networkId) { symbol = await contract.symbol(); log.debugf("symbol() =", symbol); } catch (e) { - log.errorf("symbol() failed:", e.shortMessage || e.message); + if (!signal?.aborted) { + log.errorf("symbol() failed:", e.shortMessage || e.message); + } throw new Error("Not a valid ERC-20 token (symbol() failed)."); } @@ -312,7 +315,9 @@ async function lookupTokenInfo(contractAddress, rpcUrl, networkId) { decimals = await contract.decimals(); log.debugf("decimals() =", decimals); } catch (e) { - log.errorf("decimals() failed:", e.shortMessage || e.message); + if (!signal?.aborted) { + log.errorf("decimals() failed:", e.shortMessage || e.message); + } throw new Error("Not a valid ERC-20 token (decimals() failed)."); } @@ -320,10 +325,12 @@ async function lookupTokenInfo(contractAddress, rpcUrl, networkId) { name = await contract.name(); log.debugf("name() =", name); } catch (e) { - log.warnf( - "name() failed, using symbol as name:", - e.shortMessage || e.message, - ); + if (!signal?.aborted) { + log.warnf( + "name() failed, using symbol as name:", + e.shortMessage || e.message, + ); + } name = symbol; } @@ -339,7 +346,8 @@ async function lookupTokenInfo(contractAddress, rpcUrl, networkId) { // Checks gapLimit addresses in parallel per batch. Stops when an entire // batch has no used addresses (i.e. gapLimit consecutive empty addresses). // Returns { addresses: [{ address, index }], nextIndex }. -async function scanForAddresses(xpub, rpcUrl, networkId, gapLimit = 5) { +// `signal` is as for fetchTokenBalances(). +async function scanForAddresses(xpub, rpcUrl, networkId, signal, gapLimit = 5) { log.debugf("scanForAddresses start, gapLimit:", gapLimit); const provider = getProvider(rpcUrl, networkId); const used = []; @@ -362,11 +370,13 @@ async function scanForAddresses(xpub, rpcUrl, networkId, gapLimit = 5) { ]); return { addr, index, isUsed: balance > 0n || txCount > 0 }; } catch (e) { - log.errorf( - "scanForAddresses check failed", - addr, - e.shortMessage || e.message, - ); + if (!signal?.aborted) { + log.errorf( + "scanForAddresses check failed", + addr, + e.shortMessage || e.message, + ); + } return { addr, index, isUsed: false }; } }), diff --git a/src/shared/ens.js b/src/shared/ens.js index 62e400f..355327a 100644 --- a/src/shared/ens.js +++ b/src/shared/ens.js @@ -32,7 +32,8 @@ function setCache(address, name) { localStorage.setItem(key, JSON.stringify({ name, ts: Date.now() })); } -async function resolveEnsName(address, rpcUrl, networkId) { +// `signal` is as for fetchTokenBalances() in src/shared/balances.js. +async function resolveEnsName(address, rpcUrl, networkId, signal) { const cached = getCached(address); if (cached !== undefined) return cached; @@ -42,21 +43,26 @@ async function resolveEnsName(address, rpcUrl, networkId) { setCache(address, name); return name; } catch (e) { - log.errorf( - "ENS reverse lookup failed", - address, - e.shortMessage || e.message, - ); + if (!signal?.aborted) { + log.errorf( + "ENS reverse lookup failed", + address, + e.shortMessage || e.message, + ); + } // Don't cache failures — let subsequent lookups retry return null; } } -async function resolveEnsNames(addresses, rpcUrl, networkId) { +async function resolveEnsNames(addresses, rpcUrl, networkId, signal) { const results = new Map(); await Promise.all( addresses.map(async (addr) => { - results.set(addr, await resolveEnsName(addr, rpcUrl, networkId)); + results.set( + addr, + await resolveEnsName(addr, rpcUrl, networkId, signal), + ); }), ); return results; diff --git a/tests/addressScanCancelled.test.js b/tests/addressScanCancelled.test.js new file mode 100644 index 0000000..984a51d --- /dev/null +++ b/tests/addressScanCancelled.test.js @@ -0,0 +1,121 @@ +// Creating a wallet from a recovery phrase or an extended private key does not +// report an address scan request the popup's own closing cancelled, and still +// reports one that failed while the popup was open +// (https://git.eeqj.de/sneak/AutistMask/issues/475). +// +// In the popup a cancelled request fails with the same "Failed to fetch" as a +// server that cannot be reached, so every RPC request here fails that way, and +// only the signal the popup aborts on pagehide tells the two cases apart. +// Driven against the fake elements tests/tokenLookupCancelled.test.js uses. + +const { FetchRequest, HDNodeWallet, Mnemonic } = require("ethers"); + +const PHRASE = + "abandon abandon abandon abandon abandon abandon " + + "abandon abandon abandon abandon abandon about"; +const PASSWORD = "correct horse battery staple"; + +let elements; + +function fakeElement() { + return { + value: "", + textContent: "", + style: {}, + classList: { toggle: () => {} }, + listeners: {}, + addEventListener(event, handler) { + this.listeners[event] = handler; + }, + }; +} + +// Stands in for document.getElementById(): one fake element per id. +function element(id) { + return (elements[id] ||= fakeElement()); +} + +jest.doMock("../src/popup/views/helpers", () => ({ + $: element, + showView: () => {}, + showFlash: () => {}, + goBack: () => {}, + clearViewStack: () => {}, + onViewLeave: () => {}, +})); + +// state.js reads chrome.storage.local at load, and saveState() writes it. +globalThis.chrome = { + storage: { local: { get: async () => ({}), set: async () => {} } }, +}; + +const { state } = require("../src/shared/state"); +const addWallet = require("../src/popup/views/addWallet"); + +let logged; + +beforeEach(() => { + elements = {}; + logged = []; + state.wallets = []; + state.rpcUrl = "https://rpc.example.invalid"; + state.networkId = "mainnet"; + for (const method of ["warn", "error"]) { + jest.spyOn(console, method).mockImplementation((...args) => { + logged.push(args.map(String).join(" ")); + }); + } + // What the scan found is logged at info level, which is not under test. + jest.spyOn(console, "log").mockImplementation(() => {}); + FetchRequest.registerGetUrl(async () => { + throw new TypeError("Failed to fetch"); + }); +}); + +afterEach(() => { + FetchRequest.registerGetUrl(FetchRequest.createGetUrlFunc()); + jest.restoreAllMocks(); +}); + +describe.each([ + ["a recovery phrase", "mnemonic", "wallet-mnemonic", PHRASE], + [ + "an extended private key", + "xprv", + "import-xprv-key", + HDNodeWallet.fromSeed(Mnemonic.fromPhrase(PHRASE).computeSeed()) + .extendedKey, + ], +])("creating a wallet from %s", (_name, mode, field, secret) => { + // Enters `secret` on the add-wallet screen, presses its button and waits + // for the address scan that follows. + async function create(pageClosed) { + addWallet.init({ + renderWalletList: () => {}, + doRefreshAndRender: () => {}, + pageClosed, + }); + element("tab-" + mode).listeners.click(); + element(field).value = secret; + element("add-wallet-password").value = PASSWORD; + element("add-wallet-password-confirm").value = PASSWORD; + await element("btn-add-wallet-confirm").listeners.click(); + } + + test("a scan failure while the popup is open is reported", async () => { + await create(new AbortController().signal); + expect(state.wallets).toHaveLength(1); + expect(logged).not.toEqual([]); + for (const line of logged) { + expect(line).toContain("scanForAddresses check failed"); + } + }); + + test("a scan failure once the popup has closed is not", async () => { + const pageClosed = new AbortController(); + pageClosed.abort(); + await create(pageClosed.signal); + expect(state.wallets).toHaveLength(1); + expect(logged).toEqual([]); + }); +}); diff --git a/tests/balanceRefreshCancelled.test.js b/tests/balanceRefreshCancelled.test.js index c9e56d4..5852484 100644 --- a/tests/balanceRefreshCancelled.test.js +++ b/tests/balanceRefreshCancelled.test.js @@ -1,13 +1,19 @@ -// The balance refresh does not report a request the popup's own closing -// cancelled, and still reports one that failed while the popup was open -// (https://git.eeqj.de/sneak/AutistMask/issues/218). +// The balance refresh, and the address scan after a wallet is created, do not +// report a request the popup's own closing cancelled, and still report one +// that failed while the popup was open +// (https://git.eeqj.de/sneak/AutistMask/issues/218, +// https://git.eeqj.de/sneak/AutistMask/issues/475). // // In the popup a cancelled fetch() fails with the same "Failed to fetch" as a // server that cannot be reached, so every request here fails that way, and // only the signal the popup aborts on pagehide tells the two cases apart. const { FetchRequest } = require("ethers"); -const { refreshBalances } = require("../src/shared/balances"); +const { refreshBalances, scanForAddresses } = require("../src/shared/balances"); +const { + generateMnemonic, + hdWalletFromMnemonic, +} = require("../src/shared/wallet"); const RPC_URL = "https://rpc.example.invalid"; const EXPLORER_URL = "https://explorer.example.invalid/api/v2"; @@ -24,8 +30,9 @@ beforeEach(() => { const failedToFetch = async () => { throw new TypeError("Failed to fetch"); }; - // The RPC calls (ETH balance, ENS name) and the explorer request (token - // balances) all fail the same way. + // The RPC calls (ETH balance, ENS name, and the scan's balance and + // transaction count) and the explorer request (token balances) all fail + // the same way. FetchRequest.registerGetUrl(failedToFetch); globalThis.fetch = jest.fn(failedToFetch); }); @@ -65,3 +72,24 @@ test("a failure once the popup has closed is not", async () => { await refresh(pageClosed.signal); expect(logged).toEqual([]); }); + +function scan(signal) { + // What the scan found is logged at info level, which is not under test. + jest.spyOn(console, "log").mockImplementation(() => {}); + const { xpub } = hdWalletFromMnemonic(generateMnemonic()); + return scanForAddresses(xpub, RPC_URL, "mainnet", signal); +} + +test("a scan failure while the popup is open is reported", async () => { + await scan(new AbortController().signal); + expect( + logged.some((line) => line.includes("scanForAddresses check failed")), + ).toBe(true); +}); + +test("a scan failure once the popup has closed is not", async () => { + const pageClosed = new AbortController(); + pageClosed.abort(); + await scan(pageClosed.signal); + expect(logged).toEqual([]); +}); diff --git a/tests/e2e/network.js b/tests/e2e/network.js index 86a5993..efa0fa6 100644 --- a/tests/e2e/network.js +++ b/tests/e2e/network.js @@ -420,12 +420,12 @@ const HOLD_MAX_MS = 30000; // Hold a reply open for as long as the test asks: until opts[name] is false. // -// The switch (holdGasEstimate or holdBlockscout) is read here rather than -// captured, so a test flips it on the same options object the route was -// registered with — the same pattern as seedTokenTransfer. This is the only way -// to observe a screen while its request is genuinely in flight; sampling the -// screen and hoping to win a race against the network would assert nothing on -// a slow machine. +// The switch (holdGasEstimate, holdTransactionCount or holdBlockscout) is read +// here rather than captured, so a test flips it on the same options object the +// route was registered with — the same pattern as seedTokenTransfer. This is +// the only way to observe a screen while its request is genuinely in flight; +// sampling the screen and hoping to win a race against the network would +// assert nothing on a slow machine. // // It never gives up quietly. A hold that outlives the bound is reported like // any other harness fault, because a "pending" state that stopped being @@ -560,6 +560,9 @@ async function handleRpc(route, postData, opts, report) { if (batch.some((req) => req.method === "eth_estimateGas")) { await awaitRelease(opts, "holdGasEstimate", report); } + if (batch.some((req) => req.method === "eth_getTransactionCount")) { + await awaitRelease(opts, "holdTransactionCount", report); + } const replies = batch.map((req) => rpcReply(req, opts, report)); return jsonResponse(route, Array.isArray(payload) ? replies : replies[0]); @@ -614,6 +617,8 @@ function traceEnabled(raw) { * node-side refusal. * @param {boolean} [opts.holdGasEstimate] hold every batch containing an * eth_estimateGas until this is cleared again. + * @param {boolean} [opts.holdTransactionCount] hold every batch containing an + * eth_getTransactionCount until this is cleared again. * @param {boolean} [opts.holdBlockscout] hold every Blockscout request until * this is cleared again. * @param {boolean} [opts.failTransactionList] fail every request for an diff --git a/tests/e2e/run.js b/tests/e2e/run.js index b8eec66..da66d41 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -249,8 +249,23 @@ test("the exact confirmation phrase erases the record and reloads into Welcome ( } }); -test("wallet creation through the UI reaches the main view", async (env) => { - env.phrase = await createWallet(env.page); +// The address scan that follows creating the wallet is held, so the popup +// reloads while it is in flight. As for the reloads mid-refresh further down, +// the assertion for that is the harness's own: a console.error from the page +// being reloaded fails this test (#475). +test("wallet creation through the UI reaches the main view, and reloading mid-scan reports no failure (#475)", async (env) => { + env.routeOpts.holdTransactionCount = true; + try { + const scanning = env.page.waitForRequest((r) => + (r.postData() || "").includes("eth_getTransactionCount"), + ); + env.phrase = await createWallet(env.page); + await scanning; + await env.page.reload(); + } finally { + env.routeOpts.holdTransactionCount = false; + } + await visible(env.page, "#view-main"); assert( env.phrase.split(/\s+/).length >= 12, "wallet creation did not yield a recovery phrase", @@ -700,9 +715,8 @@ test("the token contract row links to the explorer's token page (#151)", async ( // // Reloading or closing the popup makes Chrome cancel the requests it still has // open, and in the page a cancelled fetch() fails with the same "Failed to -// fetch" as a server that cannot be reached. Here rather than straight after -// wallet creation because a reload also cancels the address scan that follows -// it, which still logs (#475). +// fetch" as a server that cannot be reached. The reload during the address +// scan that follows creating a wallet is in the wallet creation test (#475). // Blockscout is held, so the home screen's transaction list and token balances // cannot have been answered when the popup reloads. The assertion is the @@ -752,6 +766,30 @@ test("a transaction list that cannot be fetched is still reported (#218)", async } }); +// The address screen's transaction list, held the same way. The popup reopens +// on the address screen, which asks for its list as it is drawn, so once the +// screen is up its request is in flight (#475). +test("reloading the popup on the address screen reports no failure (#475)", async (env) => { + await openAddressDetail(env.page); + await waitForPersisted( + env.page, + "currentView", + "address", + "before reloading the popup", + ); + env.routeOpts.holdBlockscout = true; + try { + await env.page.reload(); + await visible(env.page, "#view-address"); + await env.page.reload(); + } finally { + env.routeOpts.holdBlockscout = false; + } + await visible(env.page, "#view-address"); + // Home again, where the tests below expect to start. + await goHome(env.page); +}); + // -------------------------------------------- recovery phrase (#161) // The gear toggles, so pressing it while Settings is already up leaves it. @@ -2163,9 +2201,7 @@ function quantity(wei) { // Wait on the main view until a changed balance fixture has been picked up. // -// Not a reload: the popup re-refreshes on a 10-second timer by itself, and a -// reload on the address screen still logs the transaction list it cancels -// (#475). +// Not a reload: the popup re-refreshes on a 10-second timer by itself. // // It also deliberately settles on MAIN rather than on the address screen. // The address screen builds the send screen's token dropdown once, from the @@ -4525,6 +4561,9 @@ async function main() { ethBalanceWei: null, failGasEstimate: false, holdGasEstimate: false, + // Whether a JSON-RPC batch asking for a transaction count is held + // unanswered: the address scan after creating a wallet (#475). + holdTransactionCount: false, // Whether Blockscout requests are held unanswered, and whether an // address's transaction list fails as a network error (#218). holdBlockscout: false, diff --git a/tests/flashLine.test.js b/tests/flashLine.test.js index 8ff4a9f..c5fdfda 100644 --- a/tests/flashLine.test.js +++ b/tests/flashLine.test.js @@ -91,6 +91,7 @@ describe.each([ require("../src/popup/views/" + view).init({ doRefreshAndRender: () => {}, + pageClosed: new AbortController().signal, }); element(field).value = ADDRESS; await element(button).listeners.click(); diff --git a/tests/sendMax.test.js b/tests/sendMax.test.js index 8c71bfc..1c3f0b9 100644 --- a/tests/sendMax.test.js +++ b/tests/sendMax.test.js @@ -65,7 +65,7 @@ jest.mock("../src/shared/log", () => ({ debugf: () => {}, infof: () => {}, warnf: () => {}, - errorf: () => {}, + errorf: jest.fn(), }, // The explorer's token list, which refreshBalances() also fetches. debugFetch: jest.fn(async () => ({ @@ -143,6 +143,7 @@ const { Transaction, Wallet, formatEther } = require("ethers"); const { refreshBalances } = require("../src/shared/balances"); const { encryptWithPassword } = require("../src/shared/vault"); const { state } = require("../src/shared/state"); +const { log } = require("../src/shared/log"); const send = require("../src/popup/views/send"); const confirmTx = require("../src/popup/views/confirmTx"); @@ -226,10 +227,16 @@ function tokenRow(value, decimals = "18") { // The confirmation screen Review leads to, once shown. let confirmed = null; +// Stands in for the popup's: aborting it is the popup closing. +let pageClosed; + // Open the Send screen for `token` ("ETH" or a token address), with the // recipient entered. function openSend(token = "ETH") { - send.init({ showConfirmTx: (info) => (confirmed = info) }); + send.init({ + showConfirmTx: (info) => (confirmed = info), + pageClosed: pageClosed.signal, + }); confirmTx.init({}); send.resetSendValidation(); state.currentView = "send"; @@ -260,6 +267,8 @@ function canSend() { beforeEach(() => { elements.clear(); + log.errorf.mockClear(); + pageClosed = new AbortController(); confirmed = null; state.selectedToken = null; state.trackedTokens = []; @@ -447,6 +456,46 @@ describe("Max on an ETH send", () => { expect(text("flash-msg")).toBe(""); }); + // Holds the node's fee answer until the returned function fails it, as a + // node that cannot be reached, or a request the popup's closing + // cancelled, does. + function holdFailingFeeEstimate() { + let fail; + mockNode.feeData = new Promise((_, reject) => { + fail = () => reject(new TypeError("Failed to fetch")); + }); + return fail; + } + + test("reports a fee estimate that fails while the popup is open", async () => { + await refreshWith(BALANCE_WEI); + openSend(); + const fail = holdFailingFeeEstimate(); + const pressed = pressMax(); + fail(); + await pressed; + expect(log.errorf).toHaveBeenCalledWith( + "max amount fee estimate failed:", + "Failed to fetch", + ); + expect(text("flash-msg")).toBe( + "The network fee could not be estimated.", + ); + }); + + // https://git.eeqj.de/sneak/AutistMask/issues/475 + test("does not report a fee estimate that fails once the popup has closed", async () => { + await refreshWith(BALANCE_WEI); + openSend(); + const fail = holdFailingFeeEstimate(); + const pressed = pressMax(); + pageClosed.abort(); + fail(); + await pressed; + expect(log.errorf).not.toHaveBeenCalled(); + expect(el("send-amount").value).toBe(""); + }); + test("fills in once the held fee estimate arrives with nothing changed", async () => { await refreshWith(BALANCE_WEI); openSend(); diff --git a/tests/settingsEndpointCheck.test.js b/tests/settingsEndpointCheck.test.js index 988097f..8f73ba7 100644 --- a/tests/settingsEndpointCheck.test.js +++ b/tests/settingsEndpointCheck.test.js @@ -50,7 +50,8 @@ function element(id) { return (elements[id] ||= fakeElement()); } -function loadSettingsView() { +// `pageClosed` stands in for the popup's: aborting it is the popup closing. +function loadSettingsView(pageClosed = new AbortController()) { elements = {}; flashes = []; @@ -74,7 +75,9 @@ function loadSettingsView() { state.blockscoutUrl = SAVED_BLOCKSCOUT; require("../src/shared/log").setRuntimeDebug(true); - require("../src/popup/views/settings").init({}); + require("../src/popup/views/settings").init({ + pageClosed: pageClosed.signal, + }); } async function save(fieldId, buttonId, typed) { @@ -146,3 +149,21 @@ test("the Blockscout check of a URL with a user name and password", async () => const line = expectFailedWithoutSecrets("Blockscout validation failed"); expect(line).toContain("https://explorer.example.invalid"); }); + +// In the popup a check its own closing cancelled fails as these do, and is +// not reported (https://git.eeqj.de/sneak/AutistMask/issues/475). Nothing is +// saved either. +test("a check that fails once the popup has closed is not reported", async () => { + const pageClosed = new AbortController(); + pageClosed.abort(); + loadSettingsView(pageClosed); + await save("settings-rpc", "btn-save-rpc", RPC_UNPARSEABLE); + await save( + "settings-blockscout", + "btn-save-blockscout", + BLOCKSCOUT_WITH_PASSWORD, + ); + expect(console.error).not.toHaveBeenCalled(); + expect(state.rpcUrl).toBe(SAVED_RPC); + expect(state.blockscoutUrl).toBe(SAVED_BLOCKSCOUT); +}); diff --git a/tests/tokenLookupCancelled.test.js b/tests/tokenLookupCancelled.test.js new file mode 100644 index 0000000..6f420f7 --- /dev/null +++ b/tests/tokenLookupCancelled.test.js @@ -0,0 +1,167 @@ +// The two add-token screens do not report a token lookup the popup's own +// closing cancelled, and still report one that failed while the popup was open +// (https://git.eeqj.de/sneak/AutistMask/issues/475). +// +// In the popup a cancelled request fails with the same "Failed to fetch" as a +// server that cannot be reached, so every RPC request here fails that way, and +// only the signal the popup aborts on pagehide tells the two cases apart. The +// real lookupTokenInfo() runs, which logs a failed lookup itself before the +// screen does. Driven against the fake elements tests/flashLine.test.js uses. +// The last tests call lookupTokenInfo() directly with symbol() answered, so +// that its later calls are the ones that fail. + +const { + FetchRequest, + Interface, + toUtf8Bytes, + toUtf8String, +} = require("ethers"); +const { ERC20_ABI } = require("../src/shared/constants"); + +const ADDRESS = "0x1111111111111111111111111111111111111111"; +const RPC_URL = "https://rpc.example.invalid"; + +let elements; + +function fakeElement() { + return { + value: "", + textContent: "", + style: {}, + listeners: {}, + addEventListener(event, handler) { + this.listeners[event] = handler; + }, + }; +} + +// Stands in for document.getElementById(): one fake element per id. +function element(id) { + return (elements[id] ||= fakeElement()); +} + +jest.doMock("../src/popup/views/helpers", () => ({ + $: element, + showView: () => {}, + showFlash: () => {}, + escapeHtml: (s) => s, + goBack: () => {}, +})); + +// state.js reads chrome.storage.local at load. +globalThis.chrome = { + storage: { local: { get: async () => ({}), set: async () => {} } }, +}; + +const { state } = require("../src/shared/state"); +const { lookupTokenInfo } = require("../src/shared/balances"); + +let logged; + +beforeEach(() => { + elements = {}; + logged = []; + state.trackedTokens = []; + for (const method of ["warn", "error"]) { + jest.spyOn(console, method).mockImplementation((...args) => { + logged.push(args.map(String).join(" ")); + }); + } + FetchRequest.registerGetUrl(async () => { + throw new TypeError("Failed to fetch"); + }); +}); + +afterEach(() => { + FetchRequest.registerGetUrl(FetchRequest.createGetUrlFunc()); + jest.restoreAllMocks(); +}); + +describe.each([ + ["addToken", "add-token-address", "btn-add-token-confirm"], + [ + "settingsAddToken", + "settings-addtoken-address", + "btn-settings-addtoken-manual", + ], +])("looking up a token on %s", (view, field, button) => { + // Clicks the screen's add button for ADDRESS and waits for the lookup. + async function add(pageClosed) { + require("../src/popup/views/" + view).init({ + doRefreshAndRender: () => {}, + pageClosed, + }); + element(field).value = ADDRESS; + await element(button).listeners.click(); + } + + test("a failure while the popup is open is reported", async () => { + await add(new AbortController().signal); + expect(logged).toHaveLength(2); + expect(logged[0]).toContain("symbol() failed:"); + expect(logged[1]).toBe( + "[AutistMask] Adding token failed for " + + ADDRESS + + " Not a valid ERC-20 token (symbol() failed).", + ); + }); + + test("a failure once the popup has closed is not", async () => { + const pageClosed = new AbortController(); + pageClosed.abort(); + await add(pageClosed.signal); + expect(logged).toEqual([]); + }); +}); + +// Answers the contract call for each function named in `answers` with its +// value, and fails every other request as above. +function answer(answers) { + const erc20 = new Interface(ERC20_ABI); + FetchRequest.registerGetUrl(async (req) => { + const { id, params } = JSON.parse(toUtf8String(req.body)); + const { name } = erc20.parseTransaction({ data: params[0].data }); + if (!(name in answers)) { + throw new TypeError("Failed to fetch"); + } + const result = erc20.encodeFunctionResult(name, [answers[name]]); + return { + statusCode: 200, + statusMessage: "OK", + headers: {}, + body: toUtf8Bytes(JSON.stringify({ jsonrpc: "2.0", id, result })), + }; + }); +} + +describe.each([ + ["decimals() fails", { symbol: "TKN" }, "decimals() failed:"], + [ + "name() fails", + { symbol: "TKN", decimals: 18 }, + "name() failed, using symbol as name:", + ], +])("a token lookup where %s", (_, answers, report) => { + async function lookUp(pageClosed) { + // A token found is logged at info level, which is not under test. + jest.spyOn(console, "log").mockImplementation(() => {}); + answer(answers); + // A failed decimals() also fails the lookup, which the screens report + // as tested above. + await lookupTokenInfo(ADDRESS, RPC_URL, "mainnet", pageClosed).catch( + () => {}, + ); + } + + test("is reported while the popup is open", async () => { + await lookUp(new AbortController().signal); + expect(logged).toEqual([expect.stringContaining(report)]); + }); + + test("is not once the popup has closed", async () => { + const pageClosed = new AbortController(); + pageClosed.abort(); + await lookUp(pageClosed.signal); + expect(logged).toEqual([]); + }); +}); diff --git a/tests/transactionListCancelled.test.js b/tests/transactionListCancelled.test.js new file mode 100644 index 0000000..5e61501 --- /dev/null +++ b/tests/transactionListCancelled.test.js @@ -0,0 +1,181 @@ +// The address and token screens do not report a transaction list, or an ENS +// name lookup for the addresses in it, that the popup's own closing cancelled, +// and still report one that failed while the popup was open +// (https://git.eeqj.de/sneak/AutistMask/issues/475). +// +// In the popup a cancelled request fails with the same "Failed to fetch" as a +// server that cannot be reached, so the requests here fail that way, and only +// the signal the popup aborts on pagehide tells the two cases apart. Driven +// against a minimal DOM stub in the shape tests/timestampDisplay.test.js uses. + +// The explorer answers with mockHistory, and fails when it is null. +let mockHistory = null; +jest.mock("../src/shared/transactions", () => ({ + ...jest.requireActual("../src/shared/transactions"), + fetchRecentTransactions: async () => { + if (mockHistory === null) throw new TypeError("Failed to fetch"); + return mockHistory; + }, +})); + +// Every ENS name lookup fails. +jest.mock("../src/shared/balances", () => ({ + ...jest.requireActual("../src/shared/balances"), + getProvider: () => ({ + lookupAddress: async () => { + throw new TypeError("Failed to fetch"); + }, + }), +})); + +globalThis.chrome = { + storage: { local: { get: async () => ({}), set: async () => {} } }, +}; + +const { state } = require("../src/shared/state"); +const addressDetail = require("../src/popup/views/addressDetail"); +const addressToken = require("../src/popup/views/addressToken"); + +const ADDRESS = "0x1111111111111111111111111111111111111111"; +const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; + +// A transaction ADDRESS sent, as the history lists hold it. +function historyTx() { + return { + hash: "0x85215772ed26ea8b39c2b3b18779030487efbe0b5fd7e882592b2f62b837be84", + from: ADDRESS, + to: RECIPIENT, + value: "0.0000", + exactValue: "0.0", + rawAmount: "0", + rawUnit: "wei", + symbol: "ETH", + timestamp: 1790000000, + isError: false, + directionLabel: "Sent", + direction: "sent", + contractAddress: null, + }; +} + +function makeElement(id) { + const el = { + id, + textContent: "", + value: "", + innerHTML: "", + style: {}, + dataset: {}, + classList: { + add: () => {}, + remove: () => {}, + contains: () => false, + toggle: () => false, + }, + addEventListener: () => {}, + querySelectorAll: () => [], + appendChild: () => {}, + }; + // Views reach for .parentElement to hide whole sections. + Object.defineProperty(el, "parentElement", { + get: () => node(id + "-parent"), + }); + return el; +} + +function makeDocument() { + const els = new Map(); + return { + getElementById(id) { + // The debug banner is created on demand by helpers.js; absent + // is the state a non-debug, non-testnet popup is in. + if (id === "debug-banner") return null; + if (!els.has(id)) els.set(id, makeElement(id)); + return els.get(id); + }, + createElement: () => makeElement("created"), + addEventListener: () => {}, + body: { prepend: () => {} }, + }; +} + +function node(id) { + return globalThis.document.getElementById(id); +} + +let logged; + +beforeEach(() => { + mockHistory = null; + logged = []; + jest.spyOn(console, "error").mockImplementation((...args) => { + logged.push(args.map(String).join(" ")); + }); + globalThis.document = makeDocument(); + globalThis.window = { location: { search: "" } }; + state.wallets = [ + { + name: "Main", + type: "key", + addresses: [{ address: ADDRESS, balance: "0.0000" }], + }, + ]; + state.trackedTokens = []; + state.viewStack = []; + state.selectedWallet = 0; + state.selectedAddress = 0; + state.selectedToken = "ETH"; +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +describe.each([ + ["the address screen", addressDetail, "tx-list"], + ["the token screen", addressToken, "address-token-tx-list"], +])("the transaction list on %s", (_name, view, listId) => { + // Open the screen and wait for its transaction list to load or fail. + async function open(pageClosed) { + view.init({ pageClosed }); + view.show(); + await new Promise((resolve) => setTimeout(resolve, 0)); + } + + test("a failure while the popup is open is reported", async () => { + await open(new AbortController().signal); + expect(logged).toEqual([ + "[AutistMask] loadTransactions failed: Failed to fetch", + ]); + expect(node(listId).innerHTML).toContain( + "Failed to load transactions.", + ); + }); + + test("a failure once the popup has closed is not", async () => { + const pageClosed = new AbortController(); + pageClosed.abort(); + await open(pageClosed.signal); + expect(logged).toEqual([]); + }); + + test("a name lookup that fails while the popup is open is reported", async () => { + mockHistory = [historyTx()]; + await open(new AbortController().signal); + expect(logged).toContain( + "[AutistMask] ENS reverse lookup failed " + + RECIPIENT + + " Failed to fetch", + ); + expect(node(listId).innerHTML).toContain("tx-row"); + }); + + test("a name lookup that fails once the popup has closed is not", async () => { + mockHistory = [historyTx()]; + const pageClosed = new AbortController(); + pageClosed.abort(); + await open(pageClosed.signal); + expect(logged).toEqual([]); + expect(node(listId).innerHTML).toContain("tx-row"); + }); +}); diff --git a/tests/txStatus.test.js b/tests/txStatus.test.js index 267aabe..8da3780 100644 --- a/tests/txStatus.test.js +++ b/tests/txStatus.test.js @@ -21,7 +21,7 @@ jest.mock("../src/shared/log", () => ({ debugf: () => {}, infof: () => {}, warnf: () => {}, - errorf: () => {}, + errorf: jest.fn(), }, debugFetch: jest.fn(), setRuntimeDebug: () => {}, @@ -108,6 +108,7 @@ global.chrome = { storage }; const txStatus = require("../src/popup/views/txStatus"); const { state } = require("../src/shared/state"); const { RESTORABLE_VIEWS } = require("../src/shared/restorableViews"); +const { log } = require("../src/shared/log"); const TX_HASH = "0x85215772ed26ea8b39c2b3b18779030487efbe0b5fd7e882592b2f62b837be84"; @@ -128,16 +129,24 @@ function waitStatusText() { return getElement("wait-tx-status").textContent; } +// Stands in for the popup's: aborting it is the popup closing. +let pageClosed; + beforeEach(() => { jest.useFakeTimers(); jest.setSystemTime(new Date("2026-08-11T12:00:00Z")); elements.clear(); mockReceiptLookup.mockReset(); + log.errorf.mockClear(); state.wallets = []; state.viewData = {}; state.viewStack = []; state.currentView = null; - txStatus.init({ doRefreshAndRender: jest.fn() }); + pageClosed = new AbortController(); + txStatus.init({ + doRefreshAndRender: jest.fn(), + pageClosed: pageClosed.signal, + }); }); afterEach(() => { @@ -466,6 +475,30 @@ describe("WaitTx against an RPC that never answers", () => { }); }); +// In the popup a lookup its own closing cancelled fails as a lookup against an +// RPC that cannot be reached does, and is not reported +// (https://git.eeqj.de/sneak/AutistMask/issues/475). +describe("WaitTx when the popup closes", () => { + test("a lookup that fails while the popup is open is reported", async () => { + mockReceiptLookup.mockRejectedValue(new TypeError("Failed to fetch")); + txStatus.showWait(TX_INFO, TX_HASH); + await jest.advanceTimersByTimeAsync(10000); + expect(log.errorf).toHaveBeenCalledWith( + "poll receipt failed:", + "Failed to fetch", + ); + }); + + test("a lookup that fails once the popup has closed is not", async () => { + mockReceiptLookup.mockRejectedValue(new TypeError("Failed to fetch")); + txStatus.showWait(TX_INFO, TX_HASH); + pageClosed.abort(); + await jest.advanceTimersByTimeAsync(10000); + expect(mockReceiptLookup).toHaveBeenCalledTimes(1); + expect(log.errorf).not.toHaveBeenCalled(); + }); +}); + describe("wait-tx is a view the popup may reopen onto", () => { // The resume feature is wired through RESTORABLE_VIEWS: restoreView() // refuses any view not in the set, so dropping "wait-tx" from it kills