Skip to content

Keep replay posts alive past the page closing #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ColtenOuO wants to merge 1 commit into
base: main
Choose a base branch
from
+39 −1
Open

Keep replay posts alive past the page closing #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions tests/browser/replay-feed.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,32 @@ test("replay-feed waits out Retry-After", async () => {
delete globalThis.fetch;
}
});

test("replay-feed keeps a batch alive past the page unless it is over the keepalive budget", async () => {
const feed = await import("../../web/replay-feed.js?keepalive");
const posts = [];
globalThis.fetch = async (_url, init) => {
posts.push({ events: JSON.parse(init.body).events.length, keepalive: init.keepalive });
return { status: 204, headers: { get: () => null } };
};
try {
feed.initReplay({
state: { interviewId: "i1" },
nodes: {},
recordingEnabled: true,
consentVersion: "v1",
replayVersion: 1,
});
feed.recordReplay("lifecycle", { state: "ended", reason: "time_up" });
await feed.flushReplay();
feed.recordReplay("code", { text: "é".repeat(40_000) });
await feed.flushReplay();
assert.deepEqual(posts, [
{ events: 1, keepalive: true },
{ events: 1, keepalive: false },
]);
} finally {
feed.closeReplay();
delete globalThis.fetch;
}
});
11 changes: 10 additions & 1 deletion web/replay-feed.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const REPLAY_FLUSH_MS = 1000;
const REPLAY_MAX_BATCH = 32;
const REPLAY_RETRY_MS = 60_000;
const REPLAY_RETRY_MAX_MS = 120_000;
const REPLAY_KEEPALIVE_MAX_BYTES = 64 * 1024;

/// How often the clock and the problem heading are restated.
///
Expand Down Expand Up @@ -121,11 +122,19 @@ async function sendQueuedBatch() {
return;
}
const batch = replayQueue.splice(0, REPLAY_MAX_BATCH);
const body = JSON.stringify({ events: batch });
try {
// Kept alive so a batch already on its way survives the tab closing. The
// interview's last events are flushed as the candidate reaches the report,
// which is the moment a candidate is most likely to leave, and a plain
// fetch is cancelled with the page. Every batch rather than only the last,
// because the end may be the one inside a Retry-After window and so go
// out on the timer rather than from the call that asked for it.
const response = await fetch(`/api/interviews/${encodeURIComponent(state.interviewId)}/events`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ events: batch }),
body,
keepalive: new TextEncoder().encode(body).length <= REPLAY_KEEPALIVE_MAX_BYTES,

@jserv jserv Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keepalive is decided for the whole batch, and sendQueuedBatch takes up to REPLAY_MAX_BATCH events with no byte bound. The ending flushReplay picks up whatever is still queued (an editor snapshot of a large buffer from runTests, or a backlog left over from a Retry-After window). That makes the post carrying ended and rounds_final the one most likely to go over 64 KiB and be sent without the flag, which is exactly the case this change is for. Bound the batch by encoded size so the lifecycle events can go out in a small keepalive post. Splitting alone leaves the tail waiting on the head in flushChain, so the tail has to be sent without waiting for the head's response.

@ColtenOuO ColtenOuO Sep 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Byte-bounding the batch makes sense, and I will flush on accumulated bytes rather than only on 32 events so an editor snapshot cannot sit in the queue until the end.

In my option, sending the tail without waiting for the head looks unsafe as it stands: seq is allocated inside the insert and every read orders by it, so a small tail that wins the race hides the head's events from responseWindows.

Ordering by anything else touches the schema, replay_tail's paging and web/lib.js.

@ColtenOuO ColtenOuO Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A trade-off instead: when the final queue does not fit one keepalive post, drop the superseded editor and stage frames still queued, which are Restates kinds a review read already collapses, so the lifecycle events go out in the same ordered post and nothing is left for a tail to wait on. The only reader that loses anything is one tailing the interview live at the moment it ends, and only when the queue overflows a single post.

It doesn't completely solve the problem, but it's a simple workaround

});
// 404 is an interview whose consent has been withdrawn, and quota is an
// interview that has recorded all it may. Both mean the server will refuse
Expand Down