Skip to content

Keep replay posts alive past the page closing - #58

Open
ColtenOuO wants to merge 1 commit into
sysprog21:mainfrom
ColtenOuO:fix/replay-final-flush-keepalive
Open

ColtenOuO wants to merge 1 commit into
sysprog21:mainfrom
ColtenOuO:fix/replay-final-flush-keepalive

Conversation

@ColtenOuO

@ColtenOuO ColtenOuO commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #55. The interview's last replay events (ended, rounds_final) are flushed as the candidate reaches the report page, which is exactly when they are likely to close the tab, and a plain fetch is cancelled when the page goes away. Those events were lost and the replay just stopped.

This does not cover a tab closed while a Retry-After window is still open: the last batch is waiting on the timer and has not been posted yet, so there is no request to keep alive. It also does not cover a batch still waiting behind an earlier post or on the one-second flush timer when the tab closes.

Changes

  • Replay posts now use keepalive: true, so a batch already on its way survives the tab closing. This applies to every batch rather than only the last one, because an end that lands inside a Retry-After window goes out on the timer, not from the call that asked for it.
  • A body over the Fetch spec's 64 KiB keepalive budget is sent without the flag. Browsers refuse an oversized keepalive request outright, and that refusal would reach the offline branch and drop the batch.
  • Add a test in tests/browser/replay-feed.test.js: a normal batch is posted with keepalive, and a batch that is under 64 KiB in characters but over it in bytes is still posted, without the flag.

Summary by cubic

Fixes replay events being lost when the candidate closes the tab during the final flush.

Bug Fixes

  • In-flight posts now use keepalive: true when the body is within the 64 KiB budget.
  • Oversized bodies skip the flag to avoid the request being rejected.
  • Does not cover a tab closed during a Retry-After wait or while a batch is still queued behind an earlier post.
  • Adds a test for both keepalive and oversize behavior.

Written for commit b2b15fa. Summary will update on new commits.

Review in cubic

The interview's last replay events are flushed as the candidate reaches
the report, which is when a tab is most likely to close, and a plain
fetch is cancelled with it. Every post now asks for keepalive unless
its body is over the 64 KiB budget, which would fail the request and
drop the batch instead.
cubic-dev-ai[bot]

This comment was marked as resolved.

Comment thread web/replay-feed.js
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ events: batch }),
body,
keepalive: new TextEncoder().encode(body).length <= REPLAY_KEEPALIVE_MAX_BYTES,

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

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.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants