fix: a popup reload mid-refresh no longer logs the requests it cancels #476
@@ -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
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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
@@ -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;
|
||||
|
||||
@@ -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
@@ -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
@@ -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).
|
||||
|
||||
Reference in New Issue
Block a user