fix: WaitTx timeout no longer overwrites a rendered success screen (closes #155)
All checks were successful
check / check (push) Successful in 25s
All checks were successful
check / check (push) Successful in 25s
A poll tick that found a receipt called showSuccess() and then fell through to the elapsed check, so on the tick crossing the 60-second deadline the "Transaction Confirmed" screen was immediately replaced by "not confirmed within 60 seconds" — the user is told a confirmed transaction failed. The wait now has an explicit lifecycle. A wait id is bumped by endWait(), which is called on receipt, on timeout, when a new wait starts and when the user navigates away; every timer callback and every post-await continuation checks it, so exactly one outcome can be rendered per wait and no stale timer or in-flight receipt lookup can touch a view it no longer owns. A receipt lookup that throws is treated as "no answer this tick" rather than "no receipt": the poll returns before the deadline check and keeps running, so one transient RPC failure cannot declare a timeout. This matters most on a resumed wait, whose first poll is immediate and may already be past the deadline, where a single error would otherwise be terminal. Retrying is bounded: six consecutive failed lookups — 60 seconds at the poll cadence, the same patience the confirmation deadline gets — end the wait and report that the network could not be reached, pointing at the RPC URL in Settings. That is a different fact from the timeout, because the chain was never asked, and it says so rather than claiming the transaction did not confirm. Any lookup that answers, with a receipt or with null, resets the count. An unbounded retry would be worse than the bug it avoids: the wait is persisted, so a mistyped RPC URL would leave a wait that every popup open resumes and nothing ever ends, on a view with no exit control of its own. The wait is also persisted (state.viewData.pendingWait) and "wait-tx" is now restorable: reopening the popup resumes the poll with the elapsed counter and the deadline still measured from the original broadcast, instead of silently abandoning the wait. restoreWait() validates every field startWait() goes on to use, not just the presence of the containers — hash, a non-array object txInfo carrying a string to and a string amount, and a finite numeric broadcastTime — and returns false otherwise. txInfo.to reaches addressTitle(), which calls address.toLowerCase(), so a payload merely missing that one field would throw a TypeError out of restoreView(), which init() does not guard: the rest of popup init is skipped and wait-tx stays on screen with no back control. A non-numeric broadcastTime leaves an unexitable wait counting "NaNs". Polling stays in the popup rather than moving to the background, which would depend on setInterval surviving in an MV3 service worker. "wait-tx" is added to src/popup/restorableViews.js, and a test pins its membership. restoreView() refuses any view outside that set, so dropping the entry would kill the resume feature silently — the other tests call restoreWait() directly and never read the set. The 60-second threshold and the timeout copy are unchanged.
This commit is contained in:
@@ -16,11 +16,36 @@ const { state, saveState, currentNetwork } = require("../../shared/state");
|
||||
const { getProvider } = require("../../shared/balances");
|
||||
const { log } = require("../../shared/log");
|
||||
|
||||
// Receipt poll cadence and the deadline after which the wait is reported as
|
||||
// a timeout. Both are documented in the WaitTx section of README.md.
|
||||
const POLL_INTERVAL_MS = 10000;
|
||||
const TIMEOUT_MS = 60000;
|
||||
|
||||
// How many receipt lookups may fail in a row before the wait is ended and
|
||||
// the failure reported. A lookup that throws says nothing about the
|
||||
// transaction, so one must not end the wait — but an RPC that never answers
|
||||
// (a mistyped URL in settings is the ordinary case) must not leave the wait
|
||||
// running forever either, least of all a persisted one that every popup
|
||||
// open would resume. Six is 60 seconds at the poll cadence: the same
|
||||
// patience the confirmation deadline gets. Any lookup that answers, with a
|
||||
// receipt or with null, resets the count.
|
||||
const MAX_CONSECUTIVE_LOOKUP_FAILURES = 6;
|
||||
|
||||
let ctx;
|
||||
let elapsedTimer = null;
|
||||
let pollTimer = null;
|
||||
|
||||
function clearTimers() {
|
||||
// Identifies the wait currently on screen. Bumped by endWait(), so a timer
|
||||
// callback or an in-flight receipt lookup that outlives its wait can tell
|
||||
// that it is stale and leave the current view alone. Without it, a receipt
|
||||
// resolving after the wait has ended renders over whatever view replaced it.
|
||||
let waitId = 0;
|
||||
|
||||
// End the wait on screen: stop its timers and invalidate its pending async
|
||||
// work. Called on receipt, on timeout, when a new wait starts, and when the
|
||||
// user navigates away.
|
||||
function endWait() {
|
||||
waitId++;
|
||||
if (elapsedTimer) {
|
||||
clearInterval(elapsedTimer);
|
||||
elapsedTimer = null;
|
||||
@@ -47,8 +72,13 @@ function blockNumberHtml(blockNumber) {
|
||||
return copyableHtml(num) + etherscanLinkHtml(link);
|
||||
}
|
||||
|
||||
function showWait(txInfo, txHash) {
|
||||
clearTimers();
|
||||
// Render the wait view and start polling for the receipt. broadcastTime is
|
||||
// when the transaction was broadcast, which is what the elapsed counter and
|
||||
// the timeout deadline are both measured from; pollNow runs one lookup
|
||||
// immediately instead of waiting a full poll interval.
|
||||
function startWait(txInfo, txHash, broadcastTime, pollNow) {
|
||||
endWait();
|
||||
const id = waitId;
|
||||
|
||||
const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?";
|
||||
$("wait-tx-summary").textContent = txInfo.amount + " " + symbol;
|
||||
@@ -56,41 +86,126 @@ function showWait(txInfo, txHash) {
|
||||
$("wait-tx-hash").innerHTML = txHashHtml(txHash);
|
||||
attachCopyHandlers("view-wait-tx");
|
||||
|
||||
const broadcastTime = Date.now();
|
||||
$("wait-tx-status").textContent = "Waiting for confirmation... 0s";
|
||||
// Persisted so closing and reopening the popup resumes this wait
|
||||
// instead of silently abandoning it.
|
||||
state.viewData = {
|
||||
pendingWait: {
|
||||
txInfo: txInfo,
|
||||
hash: txHash,
|
||||
broadcastTime: broadcastTime,
|
||||
},
|
||||
};
|
||||
|
||||
elapsedTimer = setInterval(() => {
|
||||
function renderElapsed() {
|
||||
const elapsed = Math.floor((Date.now() - broadcastTime) / 1000);
|
||||
$("wait-tx-status").textContent =
|
||||
"Waiting for confirmation... " + elapsed + "s";
|
||||
}
|
||||
renderElapsed();
|
||||
|
||||
elapsedTimer = setInterval(() => {
|
||||
if (id !== waitId) return;
|
||||
renderElapsed();
|
||||
}, 1000);
|
||||
|
||||
const provider = getProvider(state.rpcUrl);
|
||||
pollTimer = setInterval(async () => {
|
||||
let consecutiveFailures = 0;
|
||||
|
||||
async function poll() {
|
||||
if (id !== waitId) return;
|
||||
let receipt = null;
|
||||
let answered = true;
|
||||
try {
|
||||
const receipt = await provider.getTransactionReceipt(txHash);
|
||||
if (receipt) {
|
||||
showSuccess(txInfo, txHash, receipt.blockNumber);
|
||||
}
|
||||
receipt = await provider.getTransactionReceipt(txHash);
|
||||
} catch (e) {
|
||||
// A thrown lookup means "no answer this tick", not "no
|
||||
// receipt": the RPC failed, the chain said nothing. Declaring
|
||||
// the timeout off it would report a confirmed transaction as
|
||||
// failed — which matters most on a resumed wait, where the
|
||||
// first poll is already past the deadline.
|
||||
answered = false;
|
||||
log.errorf("poll receipt failed:", e.message);
|
||||
}
|
||||
|
||||
const elapsed = Math.floor((Date.now() - broadcastTime) / 1000);
|
||||
if (elapsed >= 60) {
|
||||
// The lookup is async: the wait may have ended while it was in
|
||||
// flight, in which case this result must not touch the view.
|
||||
if (id !== waitId) return;
|
||||
// Exactly one outcome per wait. A receipt wins even on the tick
|
||||
// that crosses the deadline, because the transaction did confirm.
|
||||
if (receipt) {
|
||||
showSuccess(txInfo, txHash, receipt.blockNumber);
|
||||
return;
|
||||
}
|
||||
if (!answered) {
|
||||
consecutiveFailures++;
|
||||
// The failure is the user's news, and it is a different fact
|
||||
// from "the transaction did not confirm" — the chain was never
|
||||
// asked. Ending the wait here is what keeps it bounded and
|
||||
// gives the user a Done button to leave by.
|
||||
if (consecutiveFailures >= MAX_CONSECUTIVE_LOOKUP_FAILURES) {
|
||||
showError(
|
||||
txInfo,
|
||||
txHash,
|
||||
"The network could not be reached to check this transaction — " +
|
||||
MAX_CONSECUTIVE_LOOKUP_FAILURES +
|
||||
" lookups failed in a row. Check the RPC URL in Settings. The transaction may still have confirmed — check Etherscan.",
|
||||
);
|
||||
}
|
||||
// Otherwise keep polling: the next tick may answer.
|
||||
return;
|
||||
}
|
||||
consecutiveFailures = 0;
|
||||
if (Date.now() - broadcastTime >= TIMEOUT_MS) {
|
||||
showError(
|
||||
txInfo,
|
||||
txHash,
|
||||
"Transaction was not confirmed within 60 seconds. It may still confirm later \u2014 check Etherscan.",
|
||||
);
|
||||
}
|
||||
}, 10000);
|
||||
}
|
||||
|
||||
pollTimer = setInterval(poll, POLL_INTERVAL_MS);
|
||||
|
||||
showView("wait-tx");
|
||||
|
||||
if (pollNow) poll();
|
||||
}
|
||||
|
||||
function showWait(txInfo, txHash) {
|
||||
startWait(txInfo, txHash, Date.now(), false);
|
||||
}
|
||||
|
||||
// Resume a wait persisted by a previous popup session. The deadline still
|
||||
// runs from the original broadcast, so a wait that has already outlived it
|
||||
// resolves on the immediate first poll rather than restarting the clock.
|
||||
// Returns false when there is nothing resumable to resume. Every field
|
||||
// startWait() goes on to use is validated, not just the presence of the
|
||||
// containers: txInfo.to reaches addressTitle(), which calls
|
||||
// address.toLowerCase(), and txInfo.amount is rendered into the summary, so
|
||||
// an object merely missing one of them throws a TypeError out of
|
||||
// restoreView() — which init() does not guard, skipping the rest of popup
|
||||
// init and leaving wait-tx on screen with no back control. A non-numeric
|
||||
// broadcastTime leaves an unexitable wait counting "NaNs". txInfo.token and
|
||||
// txInfo.tokenSymbol are deliberately unchecked: they are compared and
|
||||
// coalesced rather than dereferenced, and tokenSymbol is null for ETH.
|
||||
function restoreWait() {
|
||||
const d = state.viewData;
|
||||
if (!d || !d.pendingWait) return false;
|
||||
const w = d.pendingWait;
|
||||
if (!w.hash) return false;
|
||||
// typeof [] is "object", so an array passes an object check.
|
||||
const info = w.txInfo;
|
||||
if (!info || typeof info !== "object" || Array.isArray(info)) return false;
|
||||
if (typeof info.to !== "string" || !info.to) return false;
|
||||
if (typeof info.amount !== "string" || !info.amount) return false;
|
||||
if (typeof w.broadcastTime !== "number" || !isFinite(w.broadcastTime)) {
|
||||
return false;
|
||||
}
|
||||
startWait(w.txInfo, w.hash, w.broadcastTime, true);
|
||||
return true;
|
||||
}
|
||||
|
||||
function showSuccess(txInfo, txHash, blockNumber) {
|
||||
clearTimers();
|
||||
endWait();
|
||||
|
||||
const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?";
|
||||
state.viewData = {
|
||||
@@ -182,7 +297,7 @@ function renderSuccess() {
|
||||
}
|
||||
|
||||
function showError(txInfo, txHash, message) {
|
||||
clearTimers();
|
||||
endWait();
|
||||
|
||||
const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?";
|
||||
state.viewData = {
|
||||
@@ -218,6 +333,9 @@ function isApprovalPopup() {
|
||||
}
|
||||
|
||||
function navigateBack() {
|
||||
// Nothing should still be polling by now, but leaving a view is the
|
||||
// point at which its timers must be gone.
|
||||
endWait();
|
||||
if (isApprovalPopup()) {
|
||||
window.close();
|
||||
return;
|
||||
@@ -242,4 +360,12 @@ function init(_ctx) {
|
||||
$("btn-error-tx-done").addEventListener("click", navigateBack);
|
||||
}
|
||||
|
||||
module.exports = { init, showWait, showError, renderSuccess, renderError };
|
||||
module.exports = {
|
||||
init,
|
||||
showWait,
|
||||
restoreWait,
|
||||
endWait,
|
||||
showError,
|
||||
renderSuccess,
|
||||
renderError,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user