fix: a popup reload mid-refresh no longer logs the requests it cancels (closes #218)
check / check (push) Successful in 2m54s
e2e / e2e-chrome (push) Successful in 6m6s
e2e / e2e-firefox (push) Successful in 2m41s

Chrome cancels a closing popup'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. 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 failure
reports check its signal first. End-to-end tests reload the popup with
Blockscout held and require nothing logged, and fail the transaction list
for real and require the failure reported; a unit test covers the balance
refresh. The address and token screens and the new-wallet address scan are
#475.

Model: opus-5-5
This commit is contained in:
2026-10-06 05:12:36 +00:00
parent 88c79e7e05
commit 226c99d0db
7 changed files with 195 additions and 21 deletions
+15
View File
@@ -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
+10
View File
@@ -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();
+3
View File
@@ -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) {
+17 -2
View File
@@ -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;
+67
View File
@@ -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([]);
});
+20 -15
View File
@@ -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
// on the same options object the route was registered with — the same
// pattern as seedTokenTransfer. This is the only way to observe the
// confirmation screen while its estimate 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 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
// 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(
"held gas estimate was never released after " +
HOLD_MAX_MS +
"ms",
);
report(name + " 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:
+63 -4
View File
@@ -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
// itself, and reloading aborts whatever fetch the home screen has open at
// that instant, which the extension reports through log.errorf and the
// harness — correctly — fails the run on.
// 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).
//
// 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).