From 0779fbdf56f2207ebffe900642cf377c2fca246f Mon Sep 17 00:00:00 2001 From: ColtenOuO Date: Thu, 17 Sep 2026 11:58:33 +0800 Subject: [PATCH] Hold replay batches until Retry-After has passed After a 429 the replay feed put the batch back and recorded when the server's window closed, then scheduled the next flush a fixed second later, and sendQueuedBatch never looked at the window. The browser asked again every second until it passed. Flushes inside the window now reschedule to its end instead of posting. --- tests/browser/replay-feed.test.js | 79 +++++++++++++++++++++++++++++++ web/interview.js | 8 ++-- web/replay-feed.js | 20 ++++++-- 3 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 tests/browser/replay-feed.test.js diff --git a/tests/browser/replay-feed.test.js b/tests/browser/replay-feed.test.js new file mode 100644 index 00000000..edec3d5c --- /dev/null +++ b/tests/browser/replay-feed.test.js @@ -0,0 +1,79 @@ +// Run with: node --test tests/browser/replay-feed.test.js +// +// The replay feed's answer to a 429, run rather than read: a fixed one-second +// retry passes every text check and still asks the server again sixty times +// inside the window it named. + +import { test } from "node:test"; +import assert from "node:assert/strict"; + +const replayFeed = await import("../../web/replay-feed.js"); + +function fakeClock() { + const real = { now: Date.now, setTimeout: globalThis.setTimeout, clearTimeout: globalThis.clearTimeout }; + let now = 0; + let nextId = 1; + const pending = new Map(); + Date.now = () => now; + globalThis.setTimeout = (fn, ms) => { + pending.set(nextId, { at: now + ms, fn }); + return nextId++; + }; + globalThis.clearTimeout = (id) => pending.delete(id); + return { + tick(ms) { + now += ms; + for (const [id, timer] of [...pending]) { + if (timer.at > now) continue; + pending.delete(id); + timer.fn(); + } + }, + restore() { + Date.now = real.now; + globalThis.setTimeout = real.setTimeout; + globalThis.clearTimeout = real.clearTimeout; + }, + }; +} + +test("replay-feed waits out Retry-After", async () => { + const clock = fakeClock(); + const posts = []; + const statuses = [429, 204]; + globalThis.fetch = async (_url, init) => { + posts.push(JSON.parse(init.body).events.length); + const status = statuses.shift(); + return { status, headers: { get: (name) => (name === "Retry-After" ? "30" : null) } }; + }; + try { + replayFeed.initReplay({ + state: { interviewId: "i1" }, + nodes: {}, + recordingEnabled: true, + consentVersion: "v1", + replayVersion: 1, + }); + replayFeed.recordReplay("lifecycle", { state: "started" }); + await replayFeed.flushReplay(); + assert.deepEqual(posts, [1], "the first batch is refused"); + + replayFeed.recordReplay("lifecycle", { state: "still_here" }); + await replayFeed.flushReplay(); + assert.deepEqual(posts, [1], "a flush inside the window sends nothing"); + + for (let second = 1; second < 30; second += 1) { + clock.tick(1000); + await new Promise(setImmediate); + } + assert.deepEqual(posts, [1], "nor does any timer before the window has passed"); + + clock.tick(1000); + await new Promise(setImmediate); + assert.deepEqual(posts, [1, 2], "then the kept batch goes, with what queued behind it"); + } finally { + replayFeed.closeReplay(); + clock.restore(); + delete globalThis.fetch; + } +}); diff --git a/web/interview.js b/web/interview.js index b86ea4b6..31201527 100644 --- a/web/interview.js +++ b/web/interview.js @@ -1473,9 +1473,11 @@ function endInterview(reason) { // who ends while it is still up would read the report status through it. globalThis.clearTimeout(frameworkHintTimer); nodes.frameworkHint.hidden = true; - // Last event, and sent rather than queued: the page is about to stop being - // the kind of page that flushes timers, and an "ended" nobody sent leaves a - // replay that just stops. + // Last event, and flushed now rather than left to the batching timer: the + // page is about to stop being the kind of page that flushes timers, and an + // "ended" nobody sent leaves a replay that just stops. Inside a Retry-After + // window the flush sends nothing and the event still waits on that timer, + // so a tab closed before the window passes loses it. recordReplay("lifecycle", { state: "ended", reason }); void flushReplay(); // The end_interview payload carries the final buffer, so drop any debounced diff --git a/web/replay-feed.js b/web/replay-feed.js index f01f7519..3a5c724e 100644 --- a/web/replay-feed.js +++ b/web/replay-feed.js @@ -56,7 +56,13 @@ export function recordReplay(kind, payload) { void flushReplay(); return; } - replayTimer ||= setTimeout(() => void flushReplay(), REPLAY_FLUSH_MS); + scheduleFlush(); +} + +function scheduleFlush() { + if (replayTimer) return; + const delay = Math.max(REPLAY_FLUSH_MS, retryAfter - Date.now()); + replayTimer = setTimeout(() => void flushReplay(), delay); } /// How long to wait after the server says it has heard enough for now. @@ -91,8 +97,10 @@ export function closeReplay() { /// that batch is kept and sent when the minute is up. /// One flush at a time, in the order they were asked for. /// -/// `recordReplay` starts one whenever the queue fills, and the interview's last -/// act awaits one. Left unserialized, a batch posted while another was still +/// `recordReplay` starts one whenever the queue fills, and the interview's end +/// asks for one without waiting on it. Inside a Retry-After window a flush +/// resolves without posting and leaves the queue to the timer that window +/// armed. Left unserialized, a batch posted while another was still /// awaiting `fetch` could commit first, and the replay would be ordered by /// whichever request the server happened to finish rather than by what the /// candidate did. Chained rather than skipped, because a caller that is told @@ -108,6 +116,10 @@ export function flushReplay() { async function sendQueuedBatch() { if (!replayQueue.length || replayClosed) return; + if (Date.now() < retryAfter) { + scheduleFlush(); + return; + } const batch = replayQueue.splice(0, REPLAY_MAX_BATCH); try { const response = await fetch(`/api/interviews/${encodeURIComponent(state.interviewId)}/events`, { @@ -154,7 +166,7 @@ async function sendQueuedBatch() { } catch { // Offline. The interview is what matters and it is still running. } - if (replayQueue.length) replayTimer ||= setTimeout(() => void flushReplay(), REPLAY_FLUSH_MS); + if (replayQueue.length) scheduleFlush(); } /// The problem heading, as the recording shows it.