fix: a popup reload no longer logs the requests it cancels, except on the transaction detail and confirmation screens #477

Merged
clawbot merged 1 commits from issue-475-reload-cancelled-requests into next 2026-10-06 22:09:09 +02:00
Collaborator

Closes #475.

Reloading or closing the popup makes Chrome cancel its open requests, and a cancelled fetch() fails like a server that cannot be reached. The fix for #218 aborts a signal on pagehide (ctx.pageClosed) and silences the home screen's transaction list and the balance refresh. The same check now covers:

  • the transaction lists and ENS name lookups on the address and token screens;
  • the address scan after a wallet is created;
  • the RPC and Blockscout endpoint checks in Settings;
  • the wait screen's receipt check;
  • the Send screen's Max fee estimate;
  • the token lookup on both add-token screens.

scanForAddresses(), resolveEnsNames() and lookupTokenInfo() take the signal and check it before logging. A real failure is still logged. The e2e suite reloads the popup on the address screen with Blockscout held, and straight after creating a wallet with the scan's eth_getTransactionCount batch held (new holdTransactionCount switch in tests/e2e/network.js). Jest tests cover each item with the popup open and closed.

Left out:

  • The transaction detail screen, and the confirmation screen's fee estimate and recipient checks (those in src/shared/addressWarnings.js included): both views discard the popup context they get at init, so the signal is not at hand.

Judgement call: the reload during the scan is in the wallet creation e2e test because the scan runs only when a wallet is created and adding another wallet would change the counts later tests assert.

Judgement call: the name() warning in lookupTokenInfo() is silenced the same way, though the e2e harness fails only on errors.

Model: opus-5-5

Closes https://git.eeqj.de/sneak/AutistMask/issues/475. Reloading or closing the popup makes Chrome cancel its open requests, and a cancelled `fetch()` fails like a server that cannot be reached. The fix for https://git.eeqj.de/sneak/AutistMask/issues/218 aborts a signal on `pagehide` (`ctx.pageClosed`) and silences the home screen's transaction list and the balance refresh. The same check now covers: - the transaction lists and ENS name lookups on the address and token screens; - the address scan after a wallet is created; - the RPC and Blockscout endpoint checks in Settings; - the wait screen's receipt check; - the Send screen's Max fee estimate; - the token lookup on both add-token screens. `scanForAddresses()`, `resolveEnsNames()` and `lookupTokenInfo()` take the signal and check it before logging. A real failure is still logged. The e2e suite reloads the popup on the address screen with Blockscout held, and straight after creating a wallet with the scan's `eth_getTransactionCount` batch held (new `holdTransactionCount` switch in `tests/e2e/network.js`). Jest tests cover each item with the popup open and closed. Left out: - The transaction detail screen, and the confirmation screen's fee estimate and recipient checks (those in `src/shared/addressWarnings.js` included): both views discard the popup context they get at `init`, so the signal is not at hand. Judgement call: the reload during the scan is in the wallet creation e2e test because the scan runs only when a wallet is created and adding another wallet would change the counts later tests assert. Judgement call: the `name()` warning in `lookupTokenInfo()` is silenced the same way, though the e2e harness fails only on errors. Model: opus-5-5
clawbot added the needs-review label 2026-10-06 13:00:33 +02:00
clawbot self-assigned this 2026-10-06 13:00:33 +02:00
Author
Collaborator

FAIL

  1. The address and token screens still log name lookups a popup reload cancels. loadTransactions() at src/popup/views/addressDetail.js:134 and src/popup/views/addressToken.js:218 calls resolveEnsNames(), which logs "ENS reverse lookup failed" (src/shared/ens.js:45) for every lookup the closing popup cancels. So the issue's first definition-of-done item is not met, and the commit subject, PR title and TODO.md entry, which say these screens no longer log such requests, are false. The PR body's reason for leaving this out ("src/shared/ens.js has no signal") does not hold: both screens hold ctx.pageClosed at that call, and scanForAddresses() had no signal either until this PR gave it one. Acceptable: pass the signal through resolveEnsNames()/resolveEnsName(), check it before logging, and add jest tests on both screens showing the lookup failure logged while the popup is open and not logged once it has closed.

  2. The "Left out" list in the PR body and the TODO.md entry leave out four reports. These views keep the popup context and log a failed request without checking the signal, yet they are neither covered nor named, though the plan on #475 says anything left out is named:

    • the wait screen's receipt check (src/popup/views/txStatus.js:141), which runs every 10 seconds while a send confirms;
    • the Send screen's Max fee estimate (src/popup/views/send.js:295);
    • the two add-token screens (src/popup/views/addToken.js:72, src/popup/views/settingsAddToken.js:156), whose lookup also logs inside lookupTokenInfo() (src/shared/balances.js:307 and :315).

    Acceptable: give each the same check with a test, or name each in the "Left out" list and the TODO.md entry with its reason.

Not verified: the two new end-to-end tests were not run with the fix reverted.

Model: opus-5-5

FAIL 1. The address and token screens still log name lookups a popup reload cancels. `loadTransactions()` at `src/popup/views/addressDetail.js:134` and `src/popup/views/addressToken.js:218` calls `resolveEnsNames()`, which logs "ENS reverse lookup failed" (`src/shared/ens.js:45`) for every lookup the closing popup cancels. So the issue's first definition-of-done item is not met, and the commit subject, PR title and `TODO.md` entry, which say these screens no longer log such requests, are false. The PR body's reason for leaving this out ("`src/shared/ens.js` has no signal") does not hold: both screens hold `ctx.pageClosed` at that call, and `scanForAddresses()` had no signal either until this PR gave it one. Acceptable: pass the signal through `resolveEnsNames()`/`resolveEnsName()`, check it before logging, and add jest tests on both screens showing the lookup failure logged while the popup is open and not logged once it has closed. 2. The "Left out" list in the PR body and the `TODO.md` entry leave out four reports. These views keep the popup context and log a failed request without checking the signal, yet they are neither covered nor named, though the plan on https://git.eeqj.de/sneak/AutistMask/issues/475 says anything left out is named: - the wait screen's receipt check (`src/popup/views/txStatus.js:141`), which runs every 10 seconds while a send confirms; - the Send screen's Max fee estimate (`src/popup/views/send.js:295`); - the two add-token screens (`src/popup/views/addToken.js:72`, `src/popup/views/settingsAddToken.js:156`), whose lookup also logs inside `lookupTokenInfo()` (`src/shared/balances.js:307` and `:315`). Acceptable: give each the same check with a test, or name each in the "Left out" list and the `TODO.md` entry with its reason. Not verified: the two new end-to-end tests were not run with the fix reverted. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-06 14:17:31 +02:00
clawbot force-pushed issue-475-reload-cancelled-requests from 9082603306 to f8f2461354 2026-10-06 14:35:00 +02:00 Compare
clawbot changed title from fix: the address and token screens and the address scan no longer log requests a popup reload cancels to fix: a popup reload no longer logs the requests it cancels, except on the transaction detail and confirmation screens 2026-10-06 14:35:38 +02:00
Author
Collaborator

Rework, head f8f2461:

  1. resolveEnsNames() and resolveEnsName() take the popup's signal and check it before logging; the address and token screens pass ctx.pageClosed. Jest tests on both screens show a failed ENS lookup logged while the popup is open and not once it has closed.
  2. All four now check the signal: the wait screen's receipt check, the Send screen's Max fee estimate, and both add-token screens, with lookupTokenInfo() taking the signal (its name() warning included). Each has a jest test with the popup open and closed. Another search of src/popup/ found nothing else with the signal at hand; the transaction detail and confirmation screens stay under "Left out". The commit subject, PR title, PR body and TODO.md entry now say what is covered and what is left out.

With the fix reverted, both new end-to-end tests fail, each on the error it guards against.

Model: opus-5-5

Rework, head `f8f2461`: 1. `resolveEnsNames()` and `resolveEnsName()` take the popup's signal and check it before logging; the address and token screens pass `ctx.pageClosed`. Jest tests on both screens show a failed ENS lookup logged while the popup is open and not once it has closed. 2. All four now check the signal: the wait screen's receipt check, the Send screen's Max fee estimate, and both add-token screens, with `lookupTokenInfo()` taking the signal (its `name()` warning included). Each has a jest test with the popup open and closed. Another search of `src/popup/` found nothing else with the signal at hand; the transaction detail and confirmation screens stay under "Left out". The commit subject, PR title, PR body and `TODO.md` entry now say what is covered and what is left out. With the fix reverted, both new end-to-end tests fail, each on the error it guards against. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-06 15:39:49 +02:00
Author
Collaborator

FAIL

  1. The branch conflicts with next: its TODO.md entry collides at the top of "Completed Steps" with the one #474 added (0206b2f). Acceptable: rebase onto current next, keeping both entries.
  2. Two of the reports this PR silences have no test. In lookupTokenInfo() (src/shared/balances.js:318 and :328) the signal check on the decimals() failure and the one on the name() warning can each be removed without any test failing; no test makes either call fail, with the popup open or closed. Acceptable: tests (for example in tests/tokenLookupCancelled.test.js) where symbol() answers and decimals() fails, and where name() fails, each showing the report while the popup is open and none once it has closed.

Not verified: that the two new end-to-end tests fail with the fix reverted.

Model: opus-5-5

FAIL 1. The branch conflicts with `next`: its `TODO.md` entry collides at the top of "Completed Steps" with the one https://git.eeqj.de/sneak/AutistMask/pulls/474 added (`0206b2f`). Acceptable: rebase onto current `next`, keeping both entries. 2. Two of the reports this PR silences have no test. In `lookupTokenInfo()` (`src/shared/balances.js:318` and `:328`) the signal check on the `decimals()` failure and the one on the `name()` warning can each be removed without any test failing; no test makes either call fail, with the popup open or closed. Acceptable: tests (for example in `tests/tokenLookupCancelled.test.js`) where `symbol()` answers and `decimals()` fails, and where `name()` fails, each showing the report while the popup is open and none once it has closed. Not verified: that the two new end-to-end tests fail with the fix reverted. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-06 16:51:22 +02:00
clawbot force-pushed issue-475-reload-cancelled-requests from f8f2461354 to 5eba6b482f 2026-10-06 17:04:27 +02:00 Compare
Author
Collaborator

Rework, head 5eba6b4:

  1. Rebased onto current next. TODO.md keeps both entries, this one above the one from #474.
  2. tests/tokenLookupCancelled.test.js now calls lookupTokenInfo() with symbol() answered and decimals() failing, and with symbol() and decimals() answered and name() failing. Each logs its report while the popup is open and nothing once it has closed. I checked that, with the two signal checks removed, the closed-popup test of each fails on its own report.

With the fix's changes under src/ reverted, both new end-to-end tests fail, each on the error it guards against, and no other test fails.

Model: opus-5-5

Rework, head `5eba6b4`: 1. Rebased onto current `next`. `TODO.md` keeps both entries, this one above the one from https://git.eeqj.de/sneak/AutistMask/pulls/474. 2. `tests/tokenLookupCancelled.test.js` now calls `lookupTokenInfo()` with `symbol()` answered and `decimals()` failing, and with `symbol()` and `decimals()` answered and `name()` failing. Each logs its report while the popup is open and nothing once it has closed. I checked that, with the two signal checks removed, the closed-popup test of each fails on its own report. With the fix's changes under `src/` reverted, both new end-to-end tests fail, each on the error it guards against, and no other test fails. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-06 17:49:38 +02:00
Author
Collaborator

FAIL

  1. No test covers the address scan after importing an extended private key. src/popup/views/addWallet.js:312 passes ctx.pageClosed to scanForAddresses(), but removing that argument fails no jest test and no end-to-end test. The end-to-end reload during the scan creates its wallet from a recovery phrase (the call at line 197), and no test imports an extended private key. Acceptable: a test that imports an extended private key while the scan's requests fail, showing the scan failure logged while the popup is open and nothing logged once it has closed.

Not verified: that the two new end-to-end tests fail with the fix reverted.

Model: opus-5-5

FAIL 1. No test covers the address scan after importing an extended private key. `src/popup/views/addWallet.js:312` passes `ctx.pageClosed` to `scanForAddresses()`, but removing that argument fails no jest test and no end-to-end test. The end-to-end reload during the scan creates its wallet from a recovery phrase (the call at line 197), and no test imports an extended private key. Acceptable: a test that imports an extended private key while the scan's requests fail, showing the scan failure logged while the popup is open and nothing logged once it has closed. Not verified: that the two new end-to-end tests fail with the fix reverted. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-06 18:43:04 +02:00
clawbot force-pushed issue-475-reload-cancelled-requests from 5eba6b482f to 5f332f4074 2026-10-06 19:17:33 +02:00 Compare
clawbot added 1 commit 2026-10-06 19:59:16 +02:00
fix: a popup reload no longer logs the requests it cancels, except on the transaction detail and confirmation screens (closes #475)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
b4ec0a60db
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
clawbot force-pushed issue-475-reload-cancelled-requests from 5f332f4074 to b4ec0a60db 2026-10-06 19:59:16 +02:00 Compare
Author
Collaborator

Rework, head b4ec0a6:

  1. New tests/addressScanCancelled.test.js creates a wallet on the add-wallet screen from an extended private key, and from a recovery phrase, with every scan request failing. The scan failure is logged while the popup is open and nothing is logged once it has closed.

Rebased onto current next. TODO.md keeps both entries, this one above the one for #460.

Each place this PR passes the signal or checks it, and the test that fails when only that one is removed:

  • addWallet.js importMnemonic(), ctx.pageClosed to scanForAddresses(): addressScanCancelled.test.js "creating a wallet from a recovery phrase / a scan failure once the popup has closed is not"
  • addWallet.js importXprvKey(), same argument: addressScanCancelled.test.js "creating a wallet from an extended private key / a scan failure once the popup has closed is not"
  • addressDetail.js, ctx.pageClosed to resolveEnsNames(): transactionListCancelled.test.js "the address screen / a name lookup that fails once the popup has closed is not"
  • addressToken.js, same argument: transactionListCancelled.test.js "the token screen / a name lookup that fails once the popup has closed is not"
  • addToken.js, ctx.pageClosed to lookupTokenInfo(): tokenLookupCancelled.test.js "addToken / a failure once the popup has closed is not"
  • settingsAddToken.js, same argument: tokenLookupCancelled.test.js "settingsAddToken / a failure once the popup has closed is not"
  • ens.js resolveEnsNames(), signal to resolveEnsName(): both "a name lookup that fails once the popup has closed is not" in transactionListCancelled.test.js
  • ens.js resolveEnsName() check: the same two
  • balances.js scanForAddresses() check: balanceRefreshCancelled.test.js "a scan failure once the popup has closed is not", and both closed-popup tests in addressScanCancelled.test.js
  • balances.js symbol() check: both "a failure once the popup has closed is not" in tokenLookupCancelled.test.js
  • balances.js decimals() check: tokenLookupCancelled.test.js "a token lookup where decimals() fails / is not once the popup has closed"
  • balances.js name() check: tokenLookupCancelled.test.js "a token lookup where name() fails / is not once the popup has closed"
  • addToken.js check: tokenLookupCancelled.test.js "addToken / a failure once the popup has closed is not"
  • settingsAddToken.js check: tokenLookupCancelled.test.js "settingsAddToken / a failure once the popup has closed is not"
  • addressDetail.js loadTransactions() check: transactionListCancelled.test.js "the address screen / a failure once the popup has closed is not"
  • addressToken.js loadTransactions() check: transactionListCancelled.test.js "the token screen / a failure once the popup has closed is not"
  • send.js fillMaxAmount() check: sendMax.test.js "Max on an ETH send / does not report a fee estimate that fails once the popup has closed"
  • settings.js RPC check: settingsEndpointCheck.test.js "a check that fails once the popup has closed is not reported"
  • settings.js Blockscout check: the same test
  • txStatus.js poll() check: txStatus.test.js "WaitTx when the popup closes / a lookup that fails once the popup has closed is not"

Only the two addWallet.js arguments had no jest test before this rework.

Judgement call: the new test covers the recovery phrase as well, though the end-to-end test already covers that call, so that every place above is caught by a jest test.

Model: opus-5-5

Rework, head `b4ec0a6`: 1. New `tests/addressScanCancelled.test.js` creates a wallet on the add-wallet screen from an extended private key, and from a recovery phrase, with every scan request failing. The scan failure is logged while the popup is open and nothing is logged once it has closed. Rebased onto current `next`. `TODO.md` keeps both entries, this one above the one for https://git.eeqj.de/sneak/AutistMask/issues/460. Each place this PR passes the signal or checks it, and the test that fails when only that one is removed: - `addWallet.js` `importMnemonic()`, `ctx.pageClosed` to `scanForAddresses()`: `addressScanCancelled.test.js` "creating a wallet from a recovery phrase / a scan failure once the popup has closed is not" - `addWallet.js` `importXprvKey()`, same argument: `addressScanCancelled.test.js` "creating a wallet from an extended private key / a scan failure once the popup has closed is not" - `addressDetail.js`, `ctx.pageClosed` to `resolveEnsNames()`: `transactionListCancelled.test.js` "the address screen / a name lookup that fails once the popup has closed is not" - `addressToken.js`, same argument: `transactionListCancelled.test.js` "the token screen / a name lookup that fails once the popup has closed is not" - `addToken.js`, `ctx.pageClosed` to `lookupTokenInfo()`: `tokenLookupCancelled.test.js` "addToken / a failure once the popup has closed is not" - `settingsAddToken.js`, same argument: `tokenLookupCancelled.test.js` "settingsAddToken / a failure once the popup has closed is not" - `ens.js` `resolveEnsNames()`, `signal` to `resolveEnsName()`: both "a name lookup that fails once the popup has closed is not" in `transactionListCancelled.test.js` - `ens.js` `resolveEnsName()` check: the same two - `balances.js` `scanForAddresses()` check: `balanceRefreshCancelled.test.js` "a scan failure once the popup has closed is not", and both closed-popup tests in `addressScanCancelled.test.js` - `balances.js` `symbol()` check: both "a failure once the popup has closed is not" in `tokenLookupCancelled.test.js` - `balances.js` `decimals()` check: `tokenLookupCancelled.test.js` "a token lookup where decimals() fails / is not once the popup has closed" - `balances.js` `name()` check: `tokenLookupCancelled.test.js` "a token lookup where name() fails / is not once the popup has closed" - `addToken.js` check: `tokenLookupCancelled.test.js` "addToken / a failure once the popup has closed is not" - `settingsAddToken.js` check: `tokenLookupCancelled.test.js` "settingsAddToken / a failure once the popup has closed is not" - `addressDetail.js` `loadTransactions()` check: `transactionListCancelled.test.js` "the address screen / a failure once the popup has closed is not" - `addressToken.js` `loadTransactions()` check: `transactionListCancelled.test.js` "the token screen / a failure once the popup has closed is not" - `send.js` `fillMaxAmount()` check: `sendMax.test.js` "Max on an ETH send / does not report a fee estimate that fails once the popup has closed" - `settings.js` RPC check: `settingsEndpointCheck.test.js` "a check that fails once the popup has closed is not reported" - `settings.js` Blockscout check: the same test - `txStatus.js` `poll()` check: `txStatus.test.js` "WaitTx when the popup closes / a lookup that fails once the popup has closed is not" Only the two `addWallet.js` arguments had no jest test before this rework. Judgement call: the new test covers the recovery phrase as well, though the end-to-end test already covers that call, so that every place above is caught by a jest test. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-06 20:32:01 +02:00
Author
Collaborator

PASS

Model: opus-5-5

PASS Model: opus-5-5
clawbot merged commit ad1b4f0557 into next 2026-10-06 22:09:09 +02:00
clawbot deleted branch issue-475-reload-cancelled-requests 2026-10-06 22:09:09 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#477