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
Contributor
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. |
lfantone
force-pushed
the
feat/react-router-web-stream-handle-request
branch
3 times, most recently
from
October 14, 2026 06:59
da1cd76 to
aa51022
Compare
lfantone
marked this pull request as ready for review
October 14, 2026 09:17
lfantone
requested review from
mydea and
s1gr1d
and removed request for
a team
October 14, 2026 09:17
lfantone
force-pushed
the
feat/react-router-web-stream-handle-request
branch
from
October 14, 2026 09:17
aa51022 to
c6da107
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
lfantone
force-pushed
the
feat/react-router-web-stream-handle-request
branch
2 times, most recently
from
October 14, 2026 17:58
c28aa9c to
b7f8d1f
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b7f8d1f. Configure here.
lfantone
force-pushed
the
feat/react-router-web-stream-handle-request
branch
2 times, most recently
from
October 19, 2026 07:21
b40312a to
d58a180
Compare
Contributor
1 similar comment
Contributor
lfantone
force-pushed
the
feat/react-router-web-stream-handle-request
branch
from
October 22, 2026 16:56
d58a180 to
22d1fe2
Compare
s1gr1d
reviewed
Oct 27, 2026
| options: RenderToReadableStreamOptions, | ||
| ) => Promise<RenderToReadableStreamResult>; | ||
|
|
||
| export interface SentryWebStreamHandleRequestOptions { |
Member
There was a problem hiding this comment.
This is very similar to SentryHandleRequestOptions: can you create a shared base interface for both so the two different types are aligned.
Contributor
Author
There was a problem hiding this comment.
Good call, added SentryHandleRequestBaseOptions, both option types extend it. Shapes are unchanged.
…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.
…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.
The abort timer can run on Node through renderToReadableStream, where a timer that is not unref'd can keep the process alive until streamTimeout while a render is still pending.
…borted The render signal was only tied to streamTimeout, so a client disconnect or platform cancellation kept rendering until the timeout. Link it to request.signal as well, and stop listening once the render has finished or failed.
…on types The Node and Web Streams handlers repeat streamTimeout, ServerRouter, botRegex and getNonce. Move them into a shared base interface so the two option types cannot drift apart. The resulting option shapes are unchanged, which a type test now guards.
lfantone
force-pushed
the
feat/react-router-web-stream-handle-request
branch
from
October 27, 2026 22:25
22d1fe2 to
5916f91
Compare
chargome
approved these changes
Oct 28, 2026
nicohrubec
pushed a commit
that referenced
this pull request
Oct 28, 2026
This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #25254 Co-authored-by: chargome <[email protected]>
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
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