Skip to content

test(nextjs): Add test case to assert no duplicate root spans for pages router - #22784

Open
logaretm wants to merge 2 commits into
developfrom
test/nextjs-pages-api-no-duplicate-root-tx
Open

test(nextjs): Add test case to assert no duplicate root spans for pages router#22784
logaretm wants to merge 2 commits into
developfrom
test/nextjs-pages-api-no-duplicate-root-tx

Conversation

@logaretm

@logaretm logaretm commented Jul 28, 2026

Copy link
Copy Markdown
Member

Adds a test to verify the changes in subsequent PRs #18394 and #22685

It asserts we do not produce duplicate root transactions, I have run this against each PR and it verified the outcome:

@logaretm
logaretm force-pushed the test/nextjs-pages-api-no-duplicate-root-tx branch from 9b79719 to 6f31175 Compare July 28, 2026 19:00
@logaretm
logaretm marked this pull request as ready for review July 28, 2026 19:01

@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 6f31175. Configure here.

const response = await request.get('/api/endpoint');
expect(await response.json()).toStrictEqual({ name: 'John Doe' });

await Promise.race([collectorPromise, new Promise(resolve => setTimeout(resolve, 6000))]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Flaky timeout-based transaction wait

Medium Severity

Flagged because the review rules call out timeouts/sleeps in telemetry waits as flake-prone. The collector always returns false, so the race always falls through to a fixed 6s setTimeout instead of a concrete signal (for example waiting for the first matching transaction, then a short follow-up window for duplicates). Under CI load that can false-fail when the first envelope is slow, or false-pass if a second root transaction arrives after the grace period.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit 6f31175. Configure here.

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.

we can probably remove the race here indeed as this can never resolve, so just doing await new Promise(...setTimeout) is probably a bit clearer 🤔

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.

Fixed in b4932be

@logaretm logaretm changed the title test/nextjs pages api no duplicate root tx test(nextjs): Add test case to assert no duplicate root spans for pages router Jul 28, 2026

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

small comment for the test but overall good coverage!

@logaretm
logaretm force-pushed the test/nextjs-pages-api-no-duplicate-root-tx branch from 6f31175 to e5c22da Compare July 29, 2026 16:08
logaretm added 2 commits July 29, 2026 12:27
Guards against duplicate root transactions for pages-router API routes:
Next.js's own BaseServer.handleRequest transaction and the one created by
wrapApiHandlerWithSentry must never both be sent for a single request.
@logaretm
logaretm force-pushed the test/nextjs-pages-api-no-duplicate-root-tx branch from e5c22da to b4932be Compare July 29, 2026 16:27
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.

2 participants