TODO.md

@@ -45,6 +45,21 @@ but the review is broader than any of them.
 
 # Completed Steps
 
+- 2026-10-06: Reloading or closing the popup mid-refresh no longer logs the
+  requests that cancels as failures
+  ([#218](https://git.eeqj.de/sneak/AutistMask/issues/218)). Chrome cancels a
+  closing page's open requests just after `pagehide`, and in the page a
+  cancelled `fetch()` fails with the same "Failed to fetch" as a server that
+  cannot be reached, so the home screen's transaction list and the balance
+  refresh logged an error for each, and the e2e suite failed on them. The popup
+  now aborts an `AbortController` on `pagehide`, and those two check its signal
+  before reporting a failure. New e2e tests reload the popup with Blockscout
+  held and require nothing logged, and fail the transaction list for real and
+  require the failure reported; `tests/balanceRefreshCancelled.test.js` covers
+  the balance refresh. The address and token screens and the address scan after
+  a new wallet still log a cancelled request
+  ([#475](https://git.eeqj.de/sneak/AutistMask/issues/475)).
+
 - 2026-10-05: `make build` no longer prints the Node `DEP0205`
   `module.register()` deprecation warning
   ([#355](https://git.eeqj.de/sneak/AutistMask/issues/355)). The call came from

src/popup/index.js

@@ -48,6 +48,14 @@ function renderWalletList() {
     home.render(ctx);
 }
 
+// Aborted when the popup page goes away, closed or reloaded. Chrome then
+// cancels the requests the page still has open, and a cancelled fetch() fails
+// with the same "Failed to fetch" as a server that cannot be reached. pagehide
+// fires first, so code that reports a failed request checks this and stays
+// silent about one the page's own closing cancelled.
+const pageClosed = new AbortController();
+window.addEventListener("pagehide", () => pageClosed.abort());
+
 let refreshInFlight = false;
 
 // The ten-second refresh init() starts, stopped when the popup moves to the
@@ -66,6 +74,7 @@ async function doRefreshAndRender() {
                 state.blockscoutUrl,
                 state.trackedTokens,
                 state.networkId,
+                pageClosed.signal,
             ),
         ]);
         state.lastBalanceRefresh = Date.now();
@@ -87,6 +96,7 @@ async function doRefreshAndRender() {
 const ctx = {
     renderWalletList,
     doRefreshAndRender,
+    pageClosed: pageClosed.signal,
     showAddWalletView: () => {
         pushCurrentView();
         addWallet.show();

src/popup/views/home.js

@@ -225,6 +225,9 @@ async function loadHomeTxs(ctx) {
         homeTxs = merged.slice(0, 25);
         renderHomeTxList(ctx);
     } catch (e) {
+        // Cancelled by the popup closing, not failed: see pageClosed in
+        // src/popup/index.js.
+        if (ctx.pageClosed.aborted) return;
         log.errorf("loadHomeTxs failed:", e.message);
         const list = $("home-tx-list");
         if (list) {

src/shared/balances.js

@@ -87,7 +87,15 @@ function rawUnits(value) {
 // way `holders` already is. Absence is never filled in here: this is the
 // upstream of every screen that displays a token amount, so a value invented
 // at this point is indistinguishable from a real one everywhere below it.
-async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) {
+//
+// `signal`, passed by the popup, is aborted when the popup closes; a request
+// that fails after that was cancelled by the closing and is not logged.
+async function fetchTokenBalances(
+    address,
+    blockscoutUrl,
+    trackedTokens,
+    signal,
+) {
     try {
         const resp = await debugFetch(
             blockscoutUrl + "/addresses/" + address + "/token-balances",
@@ -190,18 +198,22 @@ async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) {
         }
         return balances;
     } catch (e) {
-        log.errorf("fetchTokenBalances failed:", e.message);
+        if (!signal?.aborted) {
+            log.errorf("fetchTokenBalances failed:", e.message);
+        }
         return null;
     }
 }
 
 // Fetch ETH balances, ENS names, and ERC-20 token balances for all addresses.
+// `signal` is as for fetchTokenBalances().
 async function refreshBalances(
     wallets,
     rpcUrl,
     blockscoutUrl,
     trackedTokens,
     networkId,
+    signal,
 ) {
     log.debugf("refreshBalances start, rpc:", urlOrigin(rpcUrl));
     const provider = getProvider(rpcUrl, networkId);
@@ -220,6 +232,7 @@ async function refreshBalances(
                         log.debugf("ETH balance", addr.address, addr.balance);
                     })
                     .catch((e) => {
+                        if (signal?.aborted) return;
                         log.errorf(
                             "ETH balance failed",
                             addr.address,
@@ -243,6 +256,7 @@ async function refreshBalances(
                         );
                     })
                     .catch((e) => {
+                        if (signal?.aborted) return;
                         log.errorf(
                             "ENS reverse failed",
                             addr.address,
@@ -258,6 +272,7 @@ async function refreshBalances(
                     addr.address,
                     blockscoutUrl,
                     trackedTokens,
+                    signal,
                 ).then((balances) => {
                     if (balances !== null) {
                         addr.tokenBalances = balances;

tests/balanceRefreshCancelled.test.js

@@ -0,0 +1,67 @@
+// 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).
+//
+// 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 RPC_URL = "https://rpc.example.invalid";
+const EXPLORER_URL = "https://explorer.example.invalid/api/v2";
+const ADDRESS = "0x1111111111111111111111111111111111111111";
+
+const realFetch = globalThis.fetch;
+let logged;
+
+beforeEach(() => {
+    logged = [];
+    jest.spyOn(console, "error").mockImplementation((...args) => {
+        logged.push(args.map(String).join(" "));
+    });
+    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.
+    FetchRequest.registerGetUrl(failedToFetch);
+    globalThis.fetch = jest.fn(failedToFetch);
+});
+
+afterEach(() => {
+    FetchRequest.registerGetUrl(FetchRequest.createGetUrlFunc());
+    globalThis.fetch = realFetch;
+    jest.restoreAllMocks();
+});
+
+function refresh(signal) {
+    const wallets = [{ addresses: [{ address: ADDRESS }] }];
+    return refreshBalances(
+        wallets,
+        RPC_URL,
+        EXPLORER_URL,
+        [],
+        "mainnet",
+        signal,
+    );
+}
+
+test("a failure while the popup is open is reported", async () => {
+    await refresh(new AbortController().signal);
+    for (const label of [
+        "ETH balance failed",
+        "ENS reverse failed",
+        "fetchTokenBalances failed: Failed to fetch",
+    ]) {
+        expect(logged.some((line) => line.includes(label))).toBe(true);
+    }
+});
+
+test("a failure once the popup has closed is not", async () => {
+    const pageClosed = new AbortController();
+    pageClosed.abort();
+    await refresh(pageClosed.signal);
+    expect(logged).toEqual([]);
+});

tests/e2e/network.js

@@ -418,27 +418,23 @@ function sleep(ms) {
 const HOLD_POLL_MS = 25;
 const HOLD_MAX_MS = 30000;
 
-// Hold a gas estimate open for as long as the test asks.
+// Hold a reply open for as long as the test asks: until opts[name] is false.
 //
-// opts.holdGasEstimate is read here rather than captured, so a test flips it
+// The switch (holdGasEstimate or holdBlockscout) is read here rather than
-// on the same options object the route was registered with — the same
+// captured, so a test flips it on the same options object the route was
-// pattern as seedTokenTransfer. This is the only way to observe the
+// registered with — the same pattern as seedTokenTransfer. This is the only way
-// confirmation screen while its estimate is genuinely in flight; sampling
+// to observe a screen while its request is genuinely in flight; sampling the
-// the screen and hoping to win a race against the network would assert
+// screen and hoping to win a race against the network would assert nothing on
-// nothing on a slow machine.
+// 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
 // pending on its own is a green assertion about the wrong screen.
-async function awaitRelease(opts, report) {
+async function awaitRelease(opts, name, report) {
     const started = Date.now();
-    while (opts.holdGasEstimate) {
+    while (opts[name]) {
         if (Date.now() - started > HOLD_MAX_MS) {
-            report(
+            report(name + " was never released after " + HOLD_MAX_MS + "ms");
-                "held gas estimate was never released after " +
-                    HOLD_MAX_MS +
-                    "ms",
-            );
             return;
         }
         await sleep(HOLD_POLL_MS);
@@ -562,7 +558,7 @@ async function handleRpc(route, postData, opts, report) {
         return route.abort();
     }
     if (batch.some((req) => req.method === "eth_estimateGas")) {
-        await awaitRelease(opts, report);
+        await awaitRelease(opts, "holdGasEstimate", report);
     }
 
     const replies = batch.map((req) => rpcReply(req, opts, report));
@@ -618,6 +614,10 @@ 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.holdBlockscout]  hold every Blockscout request until
+ *   this is cleared again.
+ * @param {boolean} [opts.failTransactionList]  fail every request for an
+ *   address's transaction list as a network error; read at request time.
  * @param {string[]} [opts.broadcastTransactions]  every raw signed
  *   transaction handed to eth_sendRawTransaction, appended in order.
  * @param {number|string|null} [opts.tokenDecimalsOverride]  the scale
@@ -693,7 +693,12 @@ async function installNetworkStubs(ctx, opts) {
 
         // Blockscout v2
         if (p.includes("/api/v2/")) {
+            await awaitRelease(opts, "holdBlockscout", report);
             if (/\/addresses\/0x[0-9a-fA-F]{40}\/transactions$/.test(p)) {
+                // Aborted rather than answered with an error status: to the
+                // page this is a server that cannot be reached, and fetch()
+                // rejects with "Failed to fetch".
+                if (opts.failTransactionList) return route.abort();
                 const addr = blockscoutAddress(p);
                 return jsonResponse(route, {
                     items:

tests/e2e/run.js

@@ -696,6 +696,62 @@ test("the token contract row links to the explorer's token page (#151)", async (
     );
 });
 
+// ------------------------------------------- reload mid-refresh (#218)
+//
+// 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).
+
+// 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
+// harness's own: a console.error from the page being reloaded fails this test.
+test("reloading the popup mid-refresh reports no failure (#218)", async (env) => {
+    await goHome(env.page);
+    env.routeOpts.holdBlockscout = true;
+    try {
+        const inFlight = Promise.all([
+            env.page.waitForRequest((r) => r.url().endsWith("/transactions")),
+            env.page.waitForRequest((r) => r.url().endsWith("/token-balances")),
+        ]);
+        await env.page.reload();
+        await inFlight;
+        await env.page.reload();
+    } finally {
+        env.routeOpts.holdBlockscout = false;
+    }
+    await visible(env.page, "#view-main");
+});
+
+// The same "Failed to fetch" from a server that really cannot be reached is
+// still reported, so it still fails the run unless a test declares it. Chrome
+// reports the failed request itself as well, which a cancelled one does not.
+test("a transaction list that cannot be fetched is still reported (#218)", async (env) => {
+    env.errors.expect(
+        "loadHomeTxs failed",
+        /loadHomeTxs failed: Failed to fetch/,
+    );
+    env.errors.expect(
+        "Chrome's report of the failed request",
+        /Failed to load resource: net::ERR_FAILED/,
+    );
+    env.routeOpts.failTransactionList = true;
+    try {
+        // Drawn again by the next ten-second refresh.
+        await env.page.waitForFunction(
+            () =>
+                document
+                    .getElementById("home-tx-list")
+                    .textContent.includes("Failed to load transactions."),
+            null,
+            { timeout: 15000 },
+        );
+    } finally {
+        env.routeOpts.failTransactionList = false;
+    }
+});
+
 // -------------------------------------------- recovery phrase (#161)
 
 // The gear toggles, so pressing it while Settings is already up leaves it.
@@ -2107,10 +2163,9 @@ function quantity(wei) {
 
 // Wait on the main view until a changed balance fixture has been picked up.
 //
-// Deliberately not a reload: the popup re-refreshes on a 10-second timer by
+// Not a reload: the popup re-refreshes on a 10-second timer by itself, and a
-// itself, and reloading aborts whatever fetch the home screen has open at
+// reload on the address screen still logs the transaction list it cancels
-// that instant, which the extension reports through log.errorf and the
+// (#475).
-// harness — correctly — fails the run on.
 //
 // 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
@@ -4470,6 +4525,10 @@ async function main() {
         ethBalanceWei: null,
         failGasEstimate: false,
         holdGasEstimate: false,
+        // Whether Blockscout requests are held unanswered, and whether an
+        // address's transaction list fails as a network error (#218).
+        holdBlockscout: false,
+        failTransactionList: false,
         // What decimals() answers for the stub token, when it is to answer
         // something other than the value the same fixture reports through
         // Blockscout. The token that lies about its scale (#305).