fix(server): stop reporting client-aborted renders through onRequestError#2665
fix(server): stop reporting client-aborted renders through onRequestError#2665jlucaso1 wants to merge 6 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92de6f4a1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eded7a196d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…pending embed flush
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bcb9b92b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…lavor cancellation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abf4007868
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ), | ||
| cleanup, | ||
| () => { | ||
| rscEmbed.abort?.(); |
There was a problem hiding this comment.
When a document consumer disconnects while a Server Component is suspended, this only cancels rscEmbed's auxiliary branch. The other branch created for createFromReadableStream remains actively reading, and aborting the Fizz HTML render does not cancel that independent Flight reader, so the underlying RSC render can continue indefinitely and a later failure can still reach onRequestError. Fresh evidence in the current tree is the internal rscStream.tee() whose ssrStream branch is never retained for cleanup; cancel both Flight branches with the branded reason and cover a never-settling Server Component rather than one that resolves after 1.5 seconds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff7605e38e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| body: ReadableStream, | ||
| options: BuildAppPageHtmlResponseOptions, | ||
| ): Response { | ||
| body = tagConsumerCancellation(body); |
There was a problem hiding this comment.
Apply the cancellation wrapper to the HTTP-access/error-boundary response paths as well. This only tags bodies constructed through buildAppPageRscResponse and buildAppPageHtmlResponse, while renderAppPageBoundaryResponse still returns its RSC stream directly and its HTML branch uses renderAppPageHtmlResponse, which also constructs an untagged Response. If a client disconnects while a suspended custom not-found, forbidden, unauthorized, or error boundary is streaming, the cancellation therefore reaches the same RSC error handler without a ResponseAbortedError and is still reported through onRequestError; the new integration coverage exercises only a normal page.
Useful? React with 👍 / 👎.
1 participant
Closes #2664
When the response consumer goes away mid-stream (client disconnect, aborted navigation), spec compliant runtimes cancel the response body with no reason. React turns that into
Error("The render was aborted by the server without a reason.")for every pending task, and the renderonErrorreported it throughonRequestErroras if it were a real failure. On a production app with Sentry wired toonRequestErrorthis produced hundreds of noise events per hour, all with response status 200. Next.js swallows exactly this class (isAbortError/ResponseAbortedinpipe-readable.ts).Change
server/response-aborted.ts: a namedResponseAbortedError(mirroring Next'sResponseAborted) plustagConsumerCancellation(stream), a passthrough that forwards an explicit cancel reason unchanged and replaces a reason-less cancel with the tagged error. Built withhighWaterMark: 0so it stays pull-on-demand with no extra buffering.buildAppPageRscResponse/buildAppPageHtmlResponsewrap the render stream withtagConsumerCancellationright before it becomes the Response body, so the tag applies on every runtime (Node, Workers, any host that cancels the web stream).createRscOnErrorHandlerand the SSRonErrorclassify the tagged abort as expected control flow: noreportRequestError, no error meta, digest still returned for React.Real render errors are untouched: only cancellations initiated by the response consumer are reclassified, and the existing "reports server component render errors via instrumentation" test still passes.
Related cleanup: latent unhandled rejections around pipeThrough
The new coverage surfaced a latent class of unhandled rejections in the streaming path:
pipeThrough's internalpipeToloop can leave an in-flightwriter.write(or its own pipe promise) rejecting unhandled when the stream is cancelled between chunks, and the drain-detection wrappers rejected with the raw cancel reason (reproducible in ~20 lines of plain Node).deferUntilStreamConsumedand thesendAppPageResponsewrapper usedpipeThrough(new TransformStream({ flush }))only to detect clean drain; both now use a manual passthrough and fire their flush callback from thedonebranch, removing a TransformStream plus pipe machinery per response.normalizeReactFlightPreloadHints, the SSR tick-buffered embed) now run through a smallpumpThroughhelper (server/stream-pump.ts) that owns every read/write promise and propagates failures both ways, replacingpipeThrough.With the removed layers, the net per-chunk overhead of this PR is at or below main, and the affected suites now run with zero unhandled errors (they previously reported them on cancellation paths).
Tests
app-basicentry, read one chunk, cancel the body, then assert the instrumentation recorder stays empty. Fails on main with"The render was aborted by the server without a reason."recorded; passes with this change.tagConsumerCancellationtags a reason-less cancel and forwards explicit reasons unchanged.app-router-production-server(including the pre-existing instrumentation reporting test),app-flight-framing-production,app-page-dispatch,app-page-render,app-rsc-error-handler,app-rsc-errorsandapp-router-dev-serverpass with zero unhandled errors;tscis clean.