Repository navigation
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1590376. Configure here.
| startTimestamp: Date.now(), | ||
| } as utils.HandlerDataXhr); | ||
|
|
||
| expect(utils.spanToJSON(requestSpan!).attributes['url.full']).toBe(expected); |
There was a problem hiding this comment.
Tests use raw span attribute strings
Low Severity
This was flagged because the review guidelines require test assertions on span attributes to use @sentry/conventions constants rather than string literals. The new checks read url.full and url.domain as raw strings.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 1590376. Configure here.


Why:
getFullURLand the browser tracing request URL logic were resolving URLs againstlocation.origin. That breaks in two important cases:data:pages,location.origincan be the string"null".new URL()parses the base before the input, so it throws even for absolute URLs. This caused XHR spans to loseurl.fulland fetch spans to loseurl.domain.userson/dashboard/settings) were resolved against the wrong base, producing incorrecturl.fullvalues.We now resolve against
location.href, which is valid in opaque-origin contexts and preserves the correct relative URL behavior for normal pages.How to review:
Start with:
packages/browser/src/tracing/utils.tspackages/browser/src/tracing/request.tsand the regression coverage in:
packages/browser/test/tracing/request.test.tsThe key things to check are the URL normalization behavior for opaque origins and relative paths without a leading slash.
Unsure about:
Whether the empty-URL breadcrumb behavior should be handled in a separate follow-up, since it is related but intentionally not included in this fix.
Ref #13168