Skip to content

fix(node): Guard against null httpVersion in outgoing request span attributes - #20430

Merged
logaretm merged 3 commits into
developfrom
awad/js-2236-typeerror-cannot-read-properties-of-null-reading-touppercase
Apr 22, 2026
Merged

fix(node): Guard against null httpVersion in outgoing request span attributes#20430
logaretm merged 3 commits into
developfrom
awad/js-2236-typeerror-cannot-read-properties-of-null-reading-touppercase

Conversation

@logaretm

@logaretmlogaretm commented Apr 21, 2026

Copy link
Copy Markdown
Member

It appears that httpVersion can be undefined in some cases, and even tho the node types don't suggest that we do seem to treat it as such in other parts of the code so I assume we have run into this before.

This PR guards against this specific usage here as this is the only place where it wasn't guarded.

Fixes#20415

…attributes
On Node 22+ with Next.js 15, the bundled `@mswjs/interceptors` emits a
synthetic `response` event on request timeout/abort whose `IncomingMessage`
has `httpVersion === null`. `_getOutgoingRequestEndedSpanData` called
`httpVersion.toUpperCase()` unguarded, which crashed the process with an
unhandled rejection. The sibling server-side code in
`httpServerSpansIntegration` already optional-chains this attribute; this
brings the client path in line.
Fixes#20415
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CopilotAI review requested due to automatic review settings April 21, 2026 19:45

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4d64142. Configure here.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Guards Node outgoing HTTP span attribute generation against httpVersion === null to prevent crashes on synthetic response events (e.g., Node 22 + Next.js 15 + bundled @mswjs/interceptors), aligning behavior with the existing server-side HTTP spans integration.

Changes:

  • Update outgoing response span attribute construction to optional-chain httpVersion?.toUpperCase() when computing net.transport.
  • Add inline comment referencing prior similar handling and the reported issue.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadpackages/node-core/src/integrations/http/SentryHttpInstrumentation.ts Outdated
Comment threadpackages/node-core/src/integrations/http/SentryHttpInstrumentation.ts Outdated
…tion.ts
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions

github-actionsBot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize% ChangeChange
@sentry/browser25.88 kB--
@sentry/browser - with treeshaking flags24.35 kB--
@sentry/browser (incl. Tracing)43.81 kB--
@sentry/browser (incl. Tracing + Span Streaming)45.5 kB--
@sentry/browser (incl. Tracing, Profiling)48.73 kB--
@sentry/browser (incl. Tracing, Replay)82.98 kB--
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags72.5 kB--
@sentry/browser (incl. Tracing, Replay with Canvas)87.67 kB--
@sentry/browser (incl. Tracing, Replay, Feedback)99.93 kB--
@sentry/browser (incl. Feedback)42.7 kB--
@sentry/browser (incl. sendFeedback)30.55 kB--
@sentry/browser (incl. FeedbackAsync)35.55 kB--
@sentry/browser (incl. Metrics)27.16 kB--
@sentry/browser (incl. Logs)27.29 kB--
@sentry/browser (incl. Metrics & Logs)27.98 kB--
@sentry/react27.62 kB--
@sentry/react (incl. Tracing)46.05 kB--
@sentry/vue30.71 kB--
@sentry/vue (incl. Tracing)45.62 kB--
@sentry/svelte25.89 kB--
CDN Bundle28.57 kB--
CDN Bundle (incl. Tracing)46.08 kB--
CDN Bundle (incl. Logs, Metrics)29.95 kB--
CDN Bundle (incl. Tracing, Logs, Metrics)47.12 kB--
CDN Bundle (incl. Replay, Logs, Metrics)68.92 kB--
CDN Bundle (incl. Tracing, Replay)83.14 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics)84.17 kB--
CDN Bundle (incl. Tracing, Replay, Feedback)88.61 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics)89.69 kB--
CDN Bundle - uncompressed83.59 kB--
CDN Bundle (incl. Tracing) - uncompressed137.62 kB--
CDN Bundle (incl. Logs, Metrics) - uncompressed87.73 kB--
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed141.03 kB--
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed211.31 kB--
CDN Bundle (incl. Tracing, Replay) - uncompressed255.06 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed258.46 kB--
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed267.97 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed271.36 kB--
@sentry/nextjs (client)48.58 kB--
@sentry/sveltekit (client)44.22 kB--
@sentry/node-core58.35 kB+0.02%+8 B 🔺
@sentry/node175.66 kB+0.01%+12 B 🔺
@sentry/node - without tracing98.3 kB+0.02%+11 B 🔺
@sentry/aws-serverless115.33 kB+0.01%+11 B 🔺

View base workflow run

@nicohrubecnicohrubec left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we should cover this with a small unit test? Other than that lgtm :)

…` null httpVersion guard
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@logaretm

Copy link
Copy Markdown
MemberAuthor

@nicohrubec I forgot to push them, thanks! I added a few more tests now.

@logaretm
logaretm merged commit 4b4ac76 into developApr 22, 2026
712 of 723 checks passed
@logaretm
logaretm deleted the awad/js-2236-typeerror-cannot-read-properties-of-null-reading-touppercase branch April 22, 2026 20:02
@RipperZA

Copy link
Copy Markdown

It appears that httpVersion can be undefined in some cases, and even tho the node types don't suggest that we do seem to treat it as such in other parts of the code so I assume we have run into this before.

This PR guards against this specific usage here as this is the only place where it wasn't guarded.

Fixes #20415

Amazing, thanks for this quick and speedy fix! I tested this commit and it fixed the issue! Much appreciated 🚀

Sign up for freeto 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.

TypeError: Cannot read properties of null (reading 'toUpperCase') in server HTTP instrumentation when outgoing request times out (Next.js 15, Node 22)

4 participants

@logaretm@RipperZA@nicohrubec