Skip to content

fix: unlisted never-ending response bodies wedge the CDP Fetch pause indefinitely (ndjson, long-poll, grpc-web) #34623

Description

@cacieprins

Part of proxy-disabled / HTTP/2 (#33846). The follow-up PR #34593 named as "to be filed" and never filed.

Problem

shouldSkipResponseBody deny-lists exactly three provably stream-shaped responses: resourceType: 'EventSource', content-type: text/event-stream, and multipart/x-mixed-replace. Any other never-ending body — application/x-ndjson, long-poll, grpc-web, chunked streaming JSON — takes the eager path and calls Fetch.getResponseBody, which only resolves when the body completes. It never does, so the pause wedges and the run wedges with it.

This is the same failure #34470 fixed for SSE, surviving for every endless format that isn't one of those three.

There is no timeout backstop

Body transfer is deliberately exempt from the 30s response-pause timeout — cdp-fetch-transport_spec.ts:

// the pause already arrived, so a body slower than the 30s pause
// timeout must not fail the flow
await clock.tickAsync(31000)

Correct for a slow-but-finite large download; fatal for one that never ends. The wedge is unbounded, and there is a test guaranteeing nothing interrupts it.

The gap is pinned as intended behavior

cdp-fetch-transport_spec.ts asserts the predicate returns false for ndjson, commented as a documented residual gap:

// deliberately NOT skipped — documented residual gap. ndjson responses can
// also never finish, but they carry no reliable, provable content-type or
// resourceType signal the way SSE and multipart streams do.
it('does not skip application/x-ndjson', () => {  to.be.false })

So the gap is acknowledged at the predicate level, but nothing exercises the hang — no fixture, no system test, no integration test drives an endless non-SSE body through the transport. CI cannot catch or demonstrate it.

Parity impact

Not a regression (proxy-disabled never shipped), but a parity break against the MITM path, which is the bar for flipping the flag:

Same app, same spec MITM Proxy-disabled
ndjson / long-poll response completes (buffered by compression, janky) wedges forever

Degraded-but-completes → never-completes is a new failure class, not a worse grade of the existing one. #33846's exit criteria require the wrapped middleware to behave unchanged plus real-site verification; an indefinite hang fails that independently of how imperfectly the default path streams.

Repro (do this first — it is cheap)

The harness already exists: system-tests/test/server_sent_events_spec.js with a fixture server on port 3039 (system-tests/projects/e2e/cypress/e2e/server_sent_events.cy.js), already wired into the system-tests-chrome-cdp-remediated job. Add an /ndjson endpoint that writes newline-delimited JSON on an interval and never closes, plus a spec that fetches it, and run:

cd system-tests && CYPRESS_INTERNAL_DISABLE_PROXY=1 yarn test test/server_sent_events_spec.js --browser chrome

Expected today: wedges. This converts an invisible risk into a failing test and gives the regression test for whichever fix lands.

Design options

A. Idle-read timeout, format-agnostic (recommended for the minimum bar). Abandon the body read after N ms with no new bytes, then continueResponse untouched with the existing bodySkipped marker. No content-type list to maintain, degrades exactly like SSE does today, and distinguishes an idle endless stream from a slow steady download (which keeps producing bytes). Gate before implementing, mirroring how #34468 gated downloads: can an in-flight Fetch.getResponseBody be abandoned while still leaving Fetch.continueResponse valid for that pause? If not, this option dies and B becomes the minimum.

B. Extend the deny-list. Add application/x-ndjson, application/grpc-web*, etc. Cheapest to write, but whack-a-mole — every new streaming format reopens the hang, and long-poll has no distinguishing content-type at all, so it stays broken.

C. Invert to a terminability allow-list. Read eagerly only when the response proves it ends (plausible content-length); continue-without-reading otherwise. Closes the class rather than one member. Real tradeoff: moves capture loss from "rare formats" to "anything chunked," which is a large Replay fidelity regression.

D. Stream the excluded class (post-MVP capture recovery). For bodies not read eagerly, progressively read into the existing protocolManager.responseStreamReceived hand-off with a truncation cap. Note the earlier rejection of Network.streamResourceContent does not apply: that PoC measured stream-first-for-everything against materialization (~98% render-critical loss vs 0%). Here the comparison is stream-vs-skip, where partial capture strictly beats none. Brings its own decisions: truncation policy, closing the tee on an endless body, and cy.intercept becoming observe-only for the class (the browser already has the bytes).

Proposed MoSCoW

  • Must — never wedge. Any bounded outcome (skip, or fail loudly) satisfies this; it does not require solving endless bodies properly.
  • Should — cover the class rather than a hand-maintained format list.
  • Could — stream the excluded class to recover Replay capture (option D).
  • Won't, this milestone — large-body materialization ceiling and refetch/recovery machinery.

Acceptance criteria

  • An endless application/x-ndjson response does not wedge the run.
  • A long-poll response (endless, no distinguishing content-type) does not wedge the run.
  • A slow-but-finite large body still materializes and still captures — no false abandonment.
  • Skipped bodies set bodySkipped so Replay records no body rather than a false zero-length one.
  • Regression test in the *-cdp-remediated system-test job.

Related

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions