fix(server): a preview capture that never answers no longer blocks the tab#23515

External image Major · Stability & Availability · Heavy lift

Bound timed-out screenshot requests on reused CDP sessions.

captureViewport rejects after 5 seconds, but the underlying Page.captureScreenshot request remains pending. Snapshot retries reuse input.cdp. Viewer still-frame captures reuse one session for the viewer lifetime. The capture lock serializes these calls, but it does not cancel or replace the pending request. Repeated timeouts can therefore accumulate pending Playwright command callbacks.

Prevent a timed-out capture from leaving an unbounded pending request on a session that handles later captures. Limit retries until the request settles, or use a disposable or replaceable capture session. Do not detach the shared tab.cdp without replacing it.

✅ Confirmed as addressed by @DavidMendesOS

Valid in part, fixed in the next commit. The pending request is real: CDP cannot cancel Page.captureScreenshot, so a timed-out capture stays pending until the page draws or the session closes. This already happened before this PR through the snapshot's 10 s race; without the bound, though, the caller waited forever instead.

captureViewport now keeps at most one unanswered screenshot per session. A later capture waits, within its own 5 s bound, for the earlier one to settle and sends nothing until then, so timeouts no longer pile up requests. A separate or replaceable session would not help on desktop tabs: the desktop relay routes every page session to the same debugger. New test: sends no new screenshot while an earlier one is unanswered.

@DavidMendesOS The supplied implementation addresses the request-accumulation concern. unansweredCaptures permits at most one unanswered screenshot per CDP session. Later captures wait within their own 5-second deadline. The loop rechecks the pending state, so concurrent waiters cannot all send when the earlier request settles.

You are also correct that the snapshot’s existing timeout could already leave a pending request. My original wording did not distinguish that pre-existing behavior from repeated request accumulation.

The per-session gate addresses this finding without replacing or detaching the shared session. I have not run the regression tests.

📝 I couldn't check this conversation for new learnings, so no new learning was saved.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

External image Major · Stability & Availability · Quick win

Recheck the unanswered capture after each wait.

Callers that wait on the same unanswered capture resume together. Each one then sends a new screenshot without checking whether another caller has already sent one. The second send overwrites the first caller's entry. When the second screenshot settles, its cleanup removes the entry while the first screenshot is still unanswered. A later call can then send another request on that session. This breaks the one-unanswered-request rule described in the comment above unansweredCaptures.

Two callers can wait on the same entry. The snapshot path can throw before its screenshot settles, for example from the ariaSnapshot timeout or from the pageChanged() checks. void pageAndScreenshot.catch(constVoid) leaves that screenshot running. Its captureViewport call then waits on the existing entry. A later caller can wait on the same entry too.

Re-read the map in a loop so that each resumed caller waits for any newer unanswered capture before it sends.

✅ Confirmed as addressed by @DavidMendesOS

Valid, fixed with your loop in the next commit: each waiting capture re-reads the map after waking, so only the first to resume sends and the rest wait on its request. New test: lets only one waiting capture send once the unanswered one settles (three waiters produce one new send; three without the loop).

@DavidMendesOS The loop in the supplied code addresses the race. Each waiting capture re-reads unansweredCaptures before sending, so only one caller sends the next request. The reported three-waiter regression test covers this case.

📝 I couldn't check this conversation for new learnings, so no new learning was saved.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}