Skip to content

fix(sentry): Bind hub to lazy response streams#524

Draft
jan-auer wants to merge 1 commit into
mainfrom
fix/response-stream-hub
Draft

fix(sentry): Bind hub to lazy response streams#524
jan-auer wants to merge 1 commit into
mainfrom
fix/response-stream-hub

Conversation

@jan-auer

@jan-auer jan-auer commented Jun 26, 2026

Copy link
Copy Markdown
Member

When an axum handler returns a streaming body via Body::from_stream(), the IntoResponse conversion runs inside the handler (within the sentry middleware), but the body's frames are polled by hyper after middleware has unwound. This means Hub::current() inside a stream producer resolves to whatever hub the tokio worker thread happens to have — not the request's hub.

Three endpoints are affected: object_get, complete (multipart), and batch.

We introduce SentryStream<S>, a stream wrapper similar to SentryFuture in the Sentry SDK. It binds a provided hub to the stream.

A blanket middleware approach was considered, but did not work: axum's Body is type-erased, so there is no way to know whether it is a stream. Wrapping unconditionally through the middleware would strip the size_hint from all buffered responses, causing hyper to fall back to chunked transfer encoding.

This utility is a candidate for upstreaming into the sentry Rust SDK in a follow-up, alongside SentryFuture and SentryFutureExt.

Alternative to #526.

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.35%. Comparing base (824c05a) to head (acd434c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #524   +/-   ##
=======================================
  Coverage   87.35%   87.35%           
=======================================
  Files          87       88    +1     
  Lines       14086    14103   +17     
=======================================
+ Hits        12305    12320   +15     
- Misses       1781     1783    +2     
Components Coverage Δ
Rust Backend 92.16% <100.00%> (+0.01%) ⬆️
Rust Client 79.89% <ø> (ø)
Python Client 88.70% <ø> (ø)

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines +135 to +137
responses
.bind_hub(sentry::Hub::current())
.into_multipart_response(rand::random())

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one of the two locations where we now bind the hub.

To expand on the description for why this has to be done here: We could technically import http_body and then pipe the size hint and other methods through from the underlying stream. Still, it means that we now add another box around every response, and we lose any form of optimization that axum might have on eager responses.

As long as we only have two places where we stream responses back, I think this is fine. Once we have more places, we'll have to reassess this.

Axum stream bodies are polled by hyper after the middleware stack
unwinds, so Hub::current() inside a stream producer resolves to the
tokio worker thread's ambient hub rather than the request's hub.

Introduce SentryStream<S>, a pin-projected stream wrapper that
re-activates a captured Arc<Hub> via HubSwitchGuard on every
poll_next call, mirroring how SentryFuture works for futures.

Bind the hub at stream construction time in the two affected
endpoints: complete (multipart) and batch.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jan-auer jan-auer force-pushed the fix/response-stream-hub branch from 550d425 to acd434c Compare June 26, 2026 11:54
@jan-auer jan-auer marked this pull request as ready for review June 26, 2026 13:15
@jan-auer jan-auer requested a review from a team as a code owner June 26, 2026 13:15
Comment thread objectstore-server/src/endpoints/batch.rs
@jan-auer

Copy link
Copy Markdown
Member Author

Moved back to draft as I prefer the alternative PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant