Skip to content

test(react-router): Use a separate URL per loader error test in instrumentation API e2e test #25607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mydea merged 2 commits into from
Oct 28, 2026
Merged

test(react-router): Use a separate URL per loader error test in instrumentation API e2e test #25607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ec4817
test(react-router): Match error and segment span by trace in instrume…
mydea Oct 21, 2026
64e2618
test(react-router): Give each loader error test its own URL
mydea Oct 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ckages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default [
route('server-action', 'routes/performance/server-action.tsx'),
route('with-middleware', 'routes/performance/with-middleware.tsx'),
route('multi-middleware', 'routes/performance/multi-middleware.tsx'),
route('error-loader', 'routes/performance/error-loader.tsx'),
route('error-loader/:id', 'routes/performance/error-loader.tsx'),
route('error-action', 'routes/performance/error-action.tsx'),
route('error-middleware', 'routes/performance/error-middleware.tsx'),
route('lazy-route', 'routes/performance/lazy-route.tsx'),
Expand Down
6 changes: 4 additions & 2 deletions ...ications/react-router-7-framework-instrumentation/app/routes/performance/error-loader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
export function loader(): never {
throw new Error('Loader error for testing');
import type { Route } from './+types/error-loader';

export function loader({ params }: Route.LoaderArgs): never {
throw new Error(`Loader error for testing (${params.id})`);
}

export default function ErrorLoaderPage() {
Expand Down
32 changes: 20 additions & 12 deletions ...-applications/react-router-7-framework-instrumentation/tests/errors/errors.server.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expect, test } from '@playwright/test';
import type { SerializedStreamedSpan } from '@sentry-internal/test-utils';
import {
collectStreamedSpansUntilSegment,
getSpanOp,
Expand All @@ -7,17 +8,24 @@ import {
} from '@sentry-internal/test-utils';
import { APP_NAME } from '../constants';

function isErrorLoaderSegment(span: SerializedStreamedSpan, id: string): boolean {
const url = span.attributes['url.full']?.value;
return span.is_segment && typeof url === 'string' && url.endsWith(`/performance/error-loader/${id}`);
}

@cursor cursor Bot Oct 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Span matcher collides with client pageload

Medium Severity

isErrorLoaderSegment accepts any segment whose url.full ends with the test path. A client pageload segment for that same URL also matches, and streamed server spans flush later, so waitForStreamedSpan can resolve the client span. The mechanism test then fails its http.server / GET ... checks. This is flagged because the testing conventions require wait predicates to be unique when telemetry can arrive in any order.

Additional Locations (2)
Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

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


test.describe('server - instrumentation API error capture', () => {
// Each loader test hits its own URL: streamed spans flush later than errors, so a segment span
// (or error) from an earlier test on the same URL could otherwise be matched by a later test.
test('should capture loader errors with instrumentation API mechanism', async ({ page }) => {
const errorPromise = waitForError(APP_NAME, async errorEvent => {
return errorEvent.exception?.values?.[0]?.value === 'Loader error for testing';
return errorEvent.exception?.values?.[0]?.value === 'Loader error for testing (mechanism)';
});

const spanPromise = waitForStreamedSpan(APP_NAME, span => {
return span.name === 'GET /performance/error-loader' && span.is_segment;
return isErrorLoaderSegment(span, 'mechanism');
});

await page.goto(`/performance/error-loader`).catch(() => {
await page.goto(`/performance/error-loader/mechanism`).catch(() => {
// Expected to fail due to loader error
});

Expand All @@ -29,27 +37,27 @@ test.describe('server - instrumentation API error capture', () => {
values: [
{
type: 'Error',
value: 'Loader error for testing',
value: 'Loader error for testing (mechanism)',
mechanism: {
type: 'react_router.loader',
handled: false,
},
},
],
},
transaction: 'GET /performance/error-loader',
transaction: 'GET /performance/error-loader/:id',
});

// Verify the transaction was also created with correct attributes
expect(span.name).toBe('GET /performance/error-loader');
expect(span.name).toBe('GET /performance/error-loader/:id');
expect(getSpanOp(span)).toBe('http.server');
expect(span.attributes['sentry.origin']?.value).toBe('auto.http.react_router.instrumentation_api');
});

test('should include loader span in the segment even when loader throws', async ({ page }) => {
const spansPromise = collectStreamedSpansUntilSegment(APP_NAME, 'GET /performance/error-loader');
const spansPromise = collectStreamedSpansUntilSegment(APP_NAME, span => isErrorLoaderSegment(span, 'loader-span'));

await page.goto(`/performance/error-loader`).catch(() => {
await page.goto(`/performance/error-loader/loader-span`).catch(() => {
// Expected to fail due to loader error
});

Expand All @@ -69,20 +77,20 @@ test.describe('server - instrumentation API error capture', () => {

test('error and segment span should share the same trace', async ({ page }) => {
const errorPromise = waitForError(APP_NAME, async errorEvent => {
return errorEvent.exception?.values?.[0]?.value === 'Loader error for testing';
return errorEvent.exception?.values?.[0]?.value === 'Loader error for testing (same-trace)';
});

const spanPromise = waitForStreamedSpan(APP_NAME, span => {
return span.name === 'GET /performance/error-loader' && span.is_segment;
return isErrorLoaderSegment(span, 'same-trace');
});

await page.goto(`/performance/error-loader`).catch(() => {
await page.goto(`/performance/error-loader/same-trace`).catch(() => {
// Expected to fail due to loader error
});

const [error, span] = await Promise.all([errorPromise, spanPromise]);

// Error and segment span should have the same trace_id
expect(error.contexts?.trace?.trace_id).toMatch(/^[a-f0-9]{32}$/);
expect(error.contexts?.trace?.trace_id).toBe(span.trace_id);
});

Expand Down
Loading