fix(cloudflare): Keep http root span alive until streaming responses are consumed #18087
+182
−33
Conversation
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
RulaKhaled
force-pushed
the
fix-streaming
branch
from
e7d2476 to
b9d02cf
Compare
Contributor
size-limit report 📦
|
RulaKhaled
force-pushed
the
fix-streaming
branch
from
2875b4f to
eb7a5c1
Compare
Contributor
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
RulaKhaled
force-pushed
the
fix-streaming
branch
from
febee8b to
fadd2eb
Compare
RulaKhaled
force-pushed
the
fix-streaming
branch
from
d729aae to
31ecf58
Compare
RulaKhaled
requested review from
AbhiPrasad and
Lms24
and removed request for
Lms24
RulaKhaled
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+133
to
+143
| const result = await reader.read(); | ||
| done = result.done; | ||
| } | ||
| } catch { | ||
| // Stream error or cancellation - will end span in finally | ||
| } finally { | ||
| reader.releaseLock(); | ||
| span.end(); | ||
| waitUntil?.(flush(2000)); | ||
| } | ||
| })(); |
There was a problem hiding this comment.
Bug: Missing error handling for response.body.tee() call.
Severity: MEDIUM | Confidence: Low
🔍 Detailed Analysis
The call to classification.response.body.tee() is not wrapped in error handling. If tee() throws (e.g., if the body stream is locked or in an invalid state), this will result in an uncaught exception. This bypasses existing error handling, preventing proper span termination and error capture by Sentry's exception handler.
💡 Suggested Fix
Wrap the classification.response.body.tee() call in a try-catch block to handle potential TypeError exceptions gracefully.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/cloudflare/src/request.ts#L127-L143
Potential issue: The call to `classification.response.body.tee()` is not wrapped in
error handling. If `tee()` throws (e.g., if the body stream is locked or in an invalid
state), this will result in an uncaught exception. This bypasses existing error
handling, preventing proper span termination and error capture by Sentry's exception
handler.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 2881816
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.
Fixes: https://linear.app/getsentry/issue/JS-1103/spans-are-not-flushed-to-dashboard-when-using-streamtext-with-vercel
The Cloudflare request wrapper was ending the root HTTP span immediately when the handler returned a streaming Response (e.g.
result.toTextStreamResponse()). Since Vercel AI child spans only finish after the stream is consumed by the client, they were filtered out by Sentry'sisFullFinishedSpancheck, resulting in transactions with 0 spans.This PR implements a streaming response detection and handles this from within the http handler:
Created
classifyResponseStreaming()helperUpdated request wrapper
startSpan()tostartSpanManual()for manual span control