Skip to content

ref(nextjs): Remove next.js tracing escaping code - #22685

Open
mydea wants to merge 3 commits into
awad/js-1209-remove-tracing-from-pages-router-api-2from
fn/vercel-edge-remove-acs
Open

ref(nextjs): Remove next.js tracing escaping code#22685
mydea wants to merge 3 commits into
awad/js-1209-remove-tracing-from-pages-router-api-2from
fn/vercel-edge-remove-acs

Conversation

@mydea

@mydea mydea commented Jul 27, 2026

Copy link
Copy Markdown
Member

It seems that everything is working as expected without this code, so likely this is no longer needed for the versions of next.js we support...?

@cursor cursor Bot 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 321bdca. Configure here.

// to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a
// way to prevent it from actually being reported twice.)
const objectifiedErr = objectify(e);
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Duplicate API route transactions

High Severity

Removing dropNextjsRootContext and escapeNextjsTracing leaves Pages Router API requests with two root transactions. Next.js already emits a BaseServer.handleRequest span that Sentry enhances via enhanceHandleRequestRootSpan, and this wrapper still always starts another root with forceTransaction: true. Previously the Next.js span was dropped and the active span cleared so only the Sentry transaction remained. The edge wrapApiHandlerWithSentry avoids this by enhancing the existing root and starting a normal child span when one is active.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 321bdca. Configure here.

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.

seems fine?

@logaretm logaretm Jul 27, 2026

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.

Would be fine after this #18394

since we drop the startSpan call over there

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.

do we not have any tests that cover this 😅

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.

Nope, I will create a PR with that test that we can stack these two PRs on.

@mydea
mydea force-pushed the fn/vercel-edge-remove-acs branch from 321bdca to 723570e Compare July 27, 2026 12:47
@mydea
mydea requested a review from chargome July 27, 2026 13:01
@mydea mydea changed the title ref(nextjs): Remove next escaping code ref(nextjs): Remove next.js tracing escaping code Jul 27, 2026
@mydea
mydea marked this pull request as ready for review July 27, 2026 13:01
@mydea
mydea requested a review from a team as a code owner July 27, 2026 13:01
@mydea
mydea requested review from logaretm and s1gr1d and removed request for a team July 27, 2026 13:01

@s1gr1d s1gr1d 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.

As this only affects the pages router API routes, we should make sure that this is properly tested if it isn't already. Especially for duplicate/orphaned transactions (also the part that cursor mentioned)

@logaretm
logaretm force-pushed the fn/vercel-edge-remove-acs branch 2 times, most recently from d2d0c97 to c236517 Compare July 28, 2026 18:04
@logaretm
logaretm changed the base branch from develop to awad/js-1209-remove-tracing-from-pages-router-api-2 July 28, 2026 18:04
@logaretm
logaretm force-pushed the fn/vercel-edge-remove-acs branch from c236517 to af6052f Compare July 28, 2026 19:00
@logaretm

Copy link
Copy Markdown
Member

@mydea @s1gr1d I put this PR into a stack on #18394 which removes the root span that was getting dropped here.

I stacked both on #22784 to make sure we test this case.

@logaretm logaretm 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.

LGTM now

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.

3 participants