fix(server-utils): Make trace meta tag injection chunk-safe and back-pressured - #23908
Draft
JPeer264 wants to merge 1 commit into
Draft
fix(server-utils): Make trace meta tag injection chunk-safe and back-pressured#23908JPeer264 wants to merge 1 commit into
JPeer264 wants to merge 1 commit into
Conversation
…pressured
Astro, SolidStart and TanStack Start each carried their own copy of the same
head injection, and each copy had the same two defects. This moves one
implementation into `@sentry/server-utils`, which all three already depend on,
and points them at it. It lives in the shared export surface, so the entry
without diagnostics channels carries it too and edge runtimes can use it.
The scan ran over every HTML chunk on its own, so a chunk boundary inside a
quoted attribute value flipped the quote parity of the next chunk. The regex
then paired the value's closing quote with a later one, swallowed `<head>`
inside the resulting phantom string, and skipped the injection with no error.
React's Fizz writer flushes when its 2048 byte view fills and writes any longer
string on its own, so a long attribute on `<html>` puts a boundary right after
its opening quote and the trace meta tags disappear. The duplicate guard had the
same flaw: `includes('"sentry-trace"')` looked ahead within one chunk, so whether
the app's own trace meta tags were seen depended on where the split fell.
The shared injector anchors on `</head>` instead, the way `getMetaTagTransformer`
does in the react-router SDK, and carries the few characters that could still
start either token between chunks. The closing tag cannot be confused with markup
inside an attribute value, so the quote scanning goes away entirely, and
everything the head contains has been seen by the time the tags are placed, which
settles the duplicate guard too. The meta tags now sit at the end of `<head>`
rather than at the start, which is where the react-router SDK already puts them.
The body was also pumped inside the `start` callback of a `ReadableStream`, with a
`for await` loop that never consulted `desiredSize`, so it was drained as fast as
it could be produced whether or not anyone was reading the wrapped response.
TanStack's `transformStreamWithRouter` throttles itself against `desiredSize` on
the stream we consume, so that pause never fired. The shared injector carries the
consumer's backpressure through to the body being wrapped. Whether this is what
produced the `SSR stream tail exceeded maximum buffer` prerender failure in the
report is not established: on the request path React writes `</body>` last and
TanStack releases its tail only once the app stream ends, so read pacing alone
does not change how much that tail holds.
The body is pumped by hand rather than with `pipeTo`, because `pipeTo` rejects
for either side failing and a consumer that navigates away would then be reported
as an error on every aborted page load. Only a body that fails to read is
reported; a consumer going away cancels the body and stays silent.
Four smaller fixes come with it: the text decoder was never flushed, so a body
ending on an incomplete byte sequence lost its tail; holding characters back
between chunks can split a surrogate pair, which encodes each half to U+FFFD;
the upstream `content-length` was copied onto a body that had grown, which a
client or proxy would truncate; and SolidStart wrapped the tags in newlines,
which is the text node in `<head>` that React 19 whole-document hydration
rejects.
Both TanStack Start E2E apps get a `/split-head-chunk` route whose document carries
the long attribute ahead of the head. Verified against the published SDK on workerd:
the route renders a `<head>` but no trace meta tags at all. Astro writes each template
expression as its own chunk and Solid renders the shell to one string, so neither
can split inside an attribute value; SolidStart gets a plain meta tag E2E test and
Astro is already covered by its trace continuity test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
size-limit report 📦
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #23468
Astro, SolidStart and TanStack Start each carried their own copy of the same head injection, and each copy had the same two defects. This moves one implementation into
@sentry/server-utils, which all three already depend on, and points them at it. It lives in the shared export surface, so the entry without diagnostics channels carries it too and edge runtimes can use it.The scan ran over every HTML chunk on its own, so a chunk boundary inside a quoted attribute value flipped the quote parity of the next chunk. The regex then paired the value's closing quote with a later one, swallowed
<head>inside the resulting phantom string, and skipped the injection with no error. React's Fizz writer flushes when its 2048 byte view fills and writes any longer string on its own, so a long attribute on<html>puts a boundary right after its opening quote and the trace meta tags disappear. The duplicate guard had the same flaw:includes('"sentry-trace"')looked ahead within one chunk, so whether the app's own trace meta tags were seen depended on where the split fell.The shared injector anchors on
</head>instead, the waygetMetaTagTransformerdoes in the react-router SDK, and carries the few characters that could still start either token between chunks. The closing tag cannot be confused with markup inside an attribute value, so the quote scanning goes away entirely, and everything the head contains has been seen by the time the tags are placed, which settles the duplicate guard too. The meta tags now sit at the end of<head>rather than at the start, which is where the react-router SDK already puts them.The body was also pumped inside the
startcallback of aReadableStream, with afor awaitloop that never consulteddesiredSize, so it was drained as fast as it could be produced whether or not anyone was reading the wrapped response. TanStack'stransformStreamWithRouterthrottles itself againstdesiredSizeon the stream we consume, so that pause never fired. The shared injector carries the consumer's backpressure through to the body being wrapped. Whether this is what produced theSSR stream tail exceeded maximum bufferprerender failure in the report is not established: on the request path React writes</body>last and TanStack releases its tail only once the app stream ends, so read pacing alone does not change how much that tail holds.The body is pumped by hand rather than with
pipeTo, becausepipeTorejects for either side failing and a consumer that navigates away would then be reported as an error on every aborted page load. Only a body that fails to read is reported; a consumer going away cancels the body and stays silent.Four smaller fixes come with it: the text decoder was never flushed, so a body ending on an incomplete byte sequence lost its tail; holding characters back between chunks can split a surrogate pair, which encodes each half to U+FFFD; the upstream
content-lengthwas copied onto a body that had grown, which a client or proxy would truncate; and SolidStart wrapped the tags in newlines, which is the text node in<head>that React 19 whole-document hydration rejects.Both TanStack Start E2E apps get a
/split-head-chunkroute whose document carries the long attribute ahead of the head. Verified against the published SDK on workerd: the route renders a<head>but no trace meta tags at all. Astro writes each template expression as its own chunk and Solid renders the shell to one string, so neither can split inside an attribute value; SolidStart gets a plain meta tag E2E test and Astro is already covered by its trace continuity test.