Repository navigation
Conversation
chargome
reviewed
Oct 12, 2026
chargome
left a comment
chargome
left a comment
Member
There was a problem hiding this comment.
Overall looks good already, will trigger ci
Author
|
The hydrogen e2e failure was a regression from this PR: the trace tags were resolved inside the stream transform, which workerd runs outside the request context. They're now resolved up front. Fixed in 424465d. |
…dleRequest Adds a Web Streams variant of createSentryHandleRequest, accepted through an overload and exported from the cloudflare entry, and handles a </head> tag split across chunks in both the Node and Web transformers. Closes getsentry#25168
…types Resolve the trace meta tags when injectTraceMetaTags is called instead of inside the stream transform. Runtimes such as workerd run the transform outside the request's async context, where no trace data is available, so the tags were silently dropped. Also let allReady rejections propagate for bots and SPA mode, and move the handler types shared by the Node and Web Streams handlers into their own module.
lfantone
force-pushed
the
feat/react-router-web-stream-handle-request
branch
from
October 13, 2026 09:30
424465d to
85b6ae4
Compare
…ceMetaTags workerd only supports a custom TransformStream transformer from compatibility date 2024-11-11, and Hydrogen's MiniOxygen runs with an older one, so the constructor threw and no trace meta tags were injected. Use a ReadableStream with a pull handler instead, as before, while keeping the shared </head> handling.
This branch has not been deployed
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
2 participants
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.
Adds a Web Streams variant of
createSentryHandleRequest, so apps onrenderToReadableStream(non-Node runtimes, or Node withunstable_enableNodeReadableStream) no longer have to hand-composewrapSentryHandleRequestandinjectTraceMetaTags.Following the constraints from the discussion on the issue to keep it non-breaking:
SentryHandleRequestOptionsand the Node path are untouched. The Web options are a separate type, accepted through an overload and only used whenrenderToReadableStreamis passed in.</head>split across chunks was previously missed by both transformers. A shared helper now holds back only a trailing partial</head>(rather than a fixed number of characters) and flushes it at the end of the stream, so it adds as little streaming delay as possible. It is used bygetMetaTagTransformer(Node) andinjectTraceMetaTags(Web).injectTraceMetaTagsmoved to a Node-free module and is still re-exported from/cloudflare, so the public API is unchanged.react-router-8-frameworkcloudflare e2e entry now uses the new API instead of a hand-written copy of it.Fixes #25168