fix: a popup reload mid-refresh no longer logs the requests it cancels (closes #218)
check / check (push) Successful in 2m50s
e2e / e2e-chrome (push) Successful in 5m53s
e2e / e2e-firefox (push) Successful in 2m19s

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 was merged in pull request #476.
This commit is contained in:
2026-10-06 09:43:09 +02:00
parent 88c79e7e05
commit 329e64a54d
7 changed files with 195 additions and 21 deletions
+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).