Repository navigation
fix(core): Set url.scheme without colon and url.port as a number - #26284
Merged
isaacs merged 2 commits intoNov 12, 2026
Merged
Conversation
Contributor
size-limit report 📦Compared: 75614a9 (PR test merge using develop at 439a5df) Against: edae47f (develop) · base run #65716
|
Contributor
Optional E2E testsSome optional checks were not successful. They do not block the merge. 4 not successful, 56 successful. See the run.
Successful checks
|
sentry-junior
Bot
requested review from
a team,
JPeer264 and
stephanie-anderson
November 11, 2026 12:41
JPeer264
approved these changes
Nov 11, 2026
JPeer264
left a comment
JPeer264
left a comment
Member
There was a problem hiding this comment.
That is a very interesting fix. This actually got implemented already like this, but the conventions say that the new implementation is correct. Even though this looks breaking on an attribute level, aligning this to the conventions seems to be the way
isaacs
reviewed
Nov 11, 2026
isaacs
left a comment
isaacs
left a comment
Member
There was a problem hiding this comment.
Apart from the two comments, the main concern is that this PR and #26283 conflict.
dev-packages/e2e-tests/test-applications/nestjs-12/tests/spans.test.ts line 127 (on the #26283 branch). #26283 adds a RUNTIME === 'cloudflare' branch that expects 'url.scheme': 'http:' and 'url.port': { type: 'string', value: '3030' }. After this PR lands, Cloudflare sends 'http' and { type: 'integer', value: 3030 }, so the nestjs-12 (cloudflare) variant fails. That variant is optional, so CI on develop stays green, and nobody may notice the failure. The PR description mentions this, but nothing makes sure it gets done.
Let's just decide now: merge #26283 first, so this PR should target feat/nestjs-12-runtime-variants and rebase its commit to stack on top of it.
sentry-junior
Bot
force-pushed
the
fix/core-http-span-url-scheme-port
branch
from
November 11, 2026 13:18
c7b1b9e to
ad00dd6
Compare
sentry-junior
Bot
changed the base branch from
develop
to
feat/nestjs-12-runtime-variants
November 11, 2026 13:18
mydea
approved these changes
Nov 11, 2026
`getHttpSpanDetailsFromUrlObject` copied `URL.protocol` (`https:`) and `URL.port` (a string) into the span attributes. The Sentry conventions define `url.scheme` as the scheme without the colon (`https`) and `url.port` as a number, which is also what the Node `http.server` spans send. The helper builds the HTTP spans of Cloudflare, `Bun.serve`, Deno, Nitro and outgoing Node requests, so all of them change. Co-Authored-By: Isaac Schlueter <isaac.schlueter@sentry.io>
…udflare branch Assert `url.scheme` and `url.port` on an outgoing `node:http` span, add a changelog entry for the attribute value change, and drop the Cloudflare `url.scheme`/`url.port` defect from the nestjs-12 test now that the helper sends `http` and an integer port.
isaacs
force-pushed
the
fix/core-http-span-url-scheme-port
branch
from
November 11, 2026 14:28
ad00dd6 to
580a26e
Compare
isaacs
enabled stacked auto-merge (squash)
November 11, 2026 14:28
isaacs
added a commit
that referenced
this pull request
Nov 12, 2026
**Why:** Runs the `nestjs-12` suite on Bun, Deno and Cloudflare (local workerd) as optional variants next to the required Node job, with the runtime variant setup from #24598. Each runtime inits its own SDK (`@sentry/bun`, `@sentry/deno` with `--preload=@sentry/deno/import`, `@sentry/cloudflare` via `instrument.server.ts`) and adds `nestIntegration()` from `@sentry/nestjs`. New tests cover parameterized route names, per-request isolation, incoming trace continuation, outgoing `fetch` with `sentry-trace`/`baggage`, and logs. They pass on all four runtimes with no `RUNTIME` branches. All four variants are green locally through `pnpm run test:run nestjs-12 [--variant …]`. Decisions that are hard to see in the diff: - **Bun** bundles the `nest build` output with `@sentry/bun/plugin` (`bun run` gets no diagnostics channel injection, see #24612). The variant pins **Bun 1.4.4**, because Bun 1.3.14 renames classes in bundles (`let X = class X {}` becomes `class X2`). Nest span names come from class names, so 6 span tests fail on 1.3.14 (`ExampleMiddleware2`, `ParseIntPipe2`). To allow the pin, `prepare-e2e-test` gets a `bun-version` input that works like `deno-version`. - **Cloudflare** bundles the same `nest build` output (tsc emits the decorator metadata Nest DI needs) with **Vite 8**. Rollup in Vite 7 renames the classes the same way. Nest boots in the first request through `httpServerHandler` from `cloudflare:node`. - Expected differences branch on `RUNTIME`, each with a comment: the SDK name; no `http.response.status_text` on Bun, because Bun's `node:http` leaves `statusMessage` unset; the `http.server` span attributes on Cloudflare, which `withSentry` builds from the `Request`; and the two cron tests on Cloudflare, which are skipped because workerd runs no timers outside a request, so `@nestjs/schedule` jobs never fire. - On current `develop`, the Deno audit findings (broken route names, broken outgoing fetch) no longer happen, and Cloudflare errors arrive through `SentryGlobalFilter`. The Sept 2026 audit's db-span findings don't apply here because `nestjs-12` has no database. **How to review:** Start with `nestjs-12/package.json` (variants, scripts), then `bun-build.mjs`, `workers/app.ts`, and the `RUNTIME` branches in `tests/spans.test.ts`. **Unsure about:** The variants start optional, and these SDK defects are still open, so this PR only refs #24844. The `RUNTIME` branches in `tests/spans.test.ts` call them out as known defects: - `getHttpSpanDetailsFromUrlObject` (core) sets `url.scheme` with a trailing colon (`http:`) and `url.port` as a string, on Cloudflare, `Bun.serve` and Nitro server spans. #26284 fixes that. The Cloudflare `http.server` span also has no `sentry.kind` or `http.response.status_text`, which nothing tracks yet. - Bun's `node:http` leaves `res.statusMessage` unset, so Bun `http.server` spans have no `http.response.status_text`. The SDK could fall back to the standard reason phrase. Not filed yet. - The CI default Bun is still 1.3.14, which renames bundled classes. #26289 proposes making 1.4.x the default, which would remove the per-variant `bun-version` input but could change other Bun apps' results. Refs #24844 <!-- junior-request-attribution:start --> via **Isaac Schlueter**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3A6011a8eed2b7dc364fe22ff740bf6e29 --> -- [View Session](https://junior-prod.sentry.dev/conversations/agent%3A6011a8eed2b7dc364fe22ff740bf6e29) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3A6011a8eed2b7dc364fe22ff740bf6e29/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Isaac Schlueter <isaac.schlueter@sentry.io>
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.
Why:
getHttpSpanDetailsFromUrlObjectcopiedURL.protocolandURL.portas they are, so spans goturl.scheme: 'https:'andurl.port: '8080'(a string). The Sentry conventions defineurl.schemeas the bare scheme (example"https") andurl.portas a number. Nodehttp.serverspans already sendurl.scheme: 'http'and an integer port. The helper builds the HTTP spans of CloudflarewithSentry,Bun.serve, Deno, Nitro and outgoing Nodehttprequests, so their values change too: streamedurl.portbecomestype: 'integer'. The attribute keys stay the same, but saved queries on the old values stop matching, so the changelog gets an Important Changes entry.This is stacked on #26283 (merge that first). It also updates the
nestjs-12CloudflareRUNTIMEbranch from #26283:url.schemeis now the shared'http', andurl.portbecomes{ type: 'integer', value: 3030 }.How to review: The fix is two lines in
packages/core/src/utils/url.ts. The rest updates test expectations in core (including the outgoingnode:httppath), bun, cloudflare,cloudflare-integration-tests, and thedeno,cloudflare-mcpandnestjs-12e2e apps.Unsure about: Key naming, not changed here: for
kind === 'server', the helper setsurl.port, while Node server spans setserver.port(OTel HTTP server spans useserver.port). The helper already usesserver.addressfor server spans, soserver.portmay be the better key there. That rename would change product-visible data, so it needs a separate decision.Refs #24844
via Isaac Schlueter.
--
View Session [Sentry]