Skip to content

fix(node): Ensure adding sentry-trace and baggage headers via SentryHttpInstrumentation doesn't crash #16473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

andreiborza
Copy link
Member

@andreiborza andreiborza commented Jun 3, 2025

On Node > 22.10.0, when spans are off, the SentryHttpInstrumentation attempts adding sentry-trace and baggage headers to requests. Due to race-conditions, this can error in cases where the request was already sent/finished prior to setting the headers.

This fix prevents this by wrapping the logic in a try/catch.

Fixes: #16438

SentryHttpInstrumentation doesn't crash

On Node > 22.10.0, when spans are off, the `SentryHttpInstrumentation` attempts
adding `sentry-trace` and `baggage` headers to requests. Due to race-conditions,
this can error in cases where the request was already sent/finished prior to
setting the headers.

This fix prevents this by wrapping the logic in a try/catch.

Fixes: #16438
@andreiborza andreiborza requested review from Lms24, chargome and s1gr1d June 3, 2025 15:09
@andreiborza andreiborza changed the title fix(node): Ensure adding sentry-trace and baggage headers via fix(node): Ensure adding sentry-trace and baggage headers via SentryHttpInstrumentation doesn't crash Jun 3, 2025
Copy link
Contributor

github-actions bot commented Jun 3, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.99 kB - -
@sentry/browser - with treeshaking flags 23.76 kB +0.01% +1 B 🔺
@sentry/browser (incl. Tracing) 38.34 kB - -
@sentry/browser (incl. Tracing, Replay) 76.48 kB +0.01% +1 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 69.59 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 81.24 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 93.32 kB - -
@sentry/browser (incl. Feedback) 40.73 kB +0.01% +1 B 🔺
@sentry/browser (incl. sendFeedback) 28.7 kB - -
@sentry/browser (incl. FeedbackAsync) 33.59 kB - -
@sentry/react 25.76 kB - -
@sentry/react (incl. Tracing) 40.33 kB - -
@sentry/vue 28.34 kB +0.01% +1 B 🔺
@sentry/vue (incl. Tracing) 40.18 kB - -
@sentry/svelte 24.01 kB - -
CDN Bundle 25.48 kB - -
CDN Bundle (incl. Tracing) 38.52 kB - -
CDN Bundle (incl. Tracing, Replay) 74.38 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 79.8 kB - -
CDN Bundle - uncompressed 74.41 kB - -
CDN Bundle (incl. Tracing) - uncompressed 114.07 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 228.04 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 240.87 kB - -
@sentry/nextjs (client) 42 kB - -
@sentry/sveltekit (client) 38.84 kB - -
@sentry/node 150.08 kB +0.03% +44 B 🔺
@sentry/node - without tracing 98.34 kB +0.05% +47 B 🔺
@sentry/aws-serverless 124.1 kB +0.04% +41 B 🔺

View base workflow run

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

Due to race-conditions, this can error in cases where the request was already sent/finished prior to setting the headers

Doesn't this mean tracing is broken for those requests? Can we add a test that simulates those conditions?

I approved so we can unblock a merge, we probably want to do a release.

@andreiborza
Copy link
Member Author

Due to race-conditions, this can error in cases where the request was already sent/finished prior to setting the headers

Doesn't this mean tracing is broken for those requests? Can we add a test that simulates those conditions?

I approved so we can unblock a merge, we probably want to do a release.

I tried adding a test but couldn't :(

Tracing is broken for those requests yea, but there's nothing we can do. The requests are already finished by the time this reaches us.

Will merge for now.

@andreiborza andreiborza merged commit a08cae2 into develop Jun 4, 2025
116 checks passed
@andreiborza andreiborza deleted the ab/avoid-setting-headers-on-already-finished-requests branch June 4, 2025 09:49
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.

"Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client" with AWS SDK
4 participants