Skip to content

Commit b112f04

Browse files
chargomeclaude
andauthored
fix(v10/nextjs): Include basePath in request url of Pages Router errors (#25064)
Backport of: #24985 ## Differences to the original PR - `packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts`: v10 still has the older API route wrapper that starts its own `http.server` span, so the swap to `pagesRouterRequestToRequestData` lands on the `normalizedRequest` used there instead of the inline `setSDKProcessingMetadata` call. Because that span builds `url.full` and `url.path` from the same request data, on v10 those span attributes also include the `basePath` now. - `dev-packages/e2e-tests/test-applications/nextjs-15-basepath/tests/pages-router-request-url.test.ts`: v10 does not stream spans by default, so the tests wait for the `http.server` transaction and check its `request.url` instead of the streamed segment span's `url.full`. The transaction's `request.url` has no query string (it is in `query_string`), so the assertion leaves it out. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 53c4c52 commit b112f04

8 files changed

Lines changed: 159 additions & 12 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import type { NextApiRequest, NextApiResponse } from 'next';
2+
3+
export default function handler(_req: NextApiRequest, _res: NextApiResponse) {
4+
throw new Error('Pages Router API route error with basePath');
5+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default function Page() {
2+
return <p>This page should never render</p>;
3+
}
4+
5+
export async function getServerSideProps() {
6+
throw new Error('Pages Router getServerSideProps error with basePath');
7+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';
3+
4+
test('Includes the basePath in the request url of getServerSideProps errors and transactions', async ({ request }) => {
5+
const errorEventPromise = waitForError('nextjs-15-basepath', errorEvent => {
6+
return errorEvent.exception?.values?.[0]?.value === 'Pages Router getServerSideProps error with basePath';
7+
});
8+
const transactionPromise = waitForTransaction('nextjs-15-basepath', transactionEvent => {
9+
return (
10+
transactionEvent.contexts?.trace?.op === 'http.server' && !!transactionEvent.request?.url?.includes('gssp-error')
11+
);
12+
});
13+
14+
await request.get('/my-app/pages-router/gssp-error?q=1');
15+
16+
const errorEvent = await errorEventPromise;
17+
const transactionEvent = await transactionPromise;
18+
19+
expect(errorEvent.request).toMatchObject({
20+
url: 'http://localhost:3030/my-app/pages-router/gssp-error?q=1',
21+
query_string: 'q=1',
22+
});
23+
expect(transactionEvent.request).toMatchObject({
24+
url: 'http://localhost:3030/my-app/pages-router/gssp-error',
25+
query_string: 'q=1',
26+
});
27+
});
28+
29+
test('Includes the basePath in the request url of Pages Router API route errors and transactions', async ({
30+
request,
31+
}) => {
32+
const errorEventPromise = waitForError('nextjs-15-basepath', errorEvent => {
33+
return errorEvent.exception?.values?.[0]?.value === 'Pages Router API route error with basePath';
34+
});
35+
const transactionPromise = waitForTransaction('nextjs-15-basepath', transactionEvent => {
36+
return (
37+
transactionEvent.contexts?.trace?.op === 'http.server' &&
38+
!!transactionEvent.request?.url?.includes('pages-router-api-error')
39+
);
40+
});
41+
42+
await request.get('/my-app/api/pages-router-api-error');
43+
44+
const errorEvent = await errorEventPromise;
45+
const transactionEvent = await transactionPromise;
46+
47+
expect(errorEvent.request?.url).toBe('http://localhost:3030/my-app/api/pages-router-api-error');
48+
expect(transactionEvent.request?.url).toBe('http://localhost:3030/my-app/api/pages-router-api-error');
49+
});

‎packages/nextjs/src/common/pages-router-instrumentation/_error.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
1-
import {
2-
captureException,
3-
getIsolationScope,
4-
httpRequestToRequestData,
5-
isAlreadyCaptured,
6-
withScope,
7-
} from '@sentry/core';
1+
import { captureException, getIsolationScope, isAlreadyCaptured, withScope } from '@sentry/core';
82
import type { NextPageContext } from 'next';
93
import { flushSafelyWithTimeout, waitUntil } from '../utils/responseEnd';
4+
import { pagesRouterRequestToRequestData } from '../utils/pagesRouterRequestToRequestData';
105

116
type ContextOrProps = {
127
req?: NextPageContext['req'];
@@ -62,7 +57,7 @@ export async function captureUnderscoreErrorException(contextOrProps: ContextOrP
6257

6358
const eventId = withScope(scope => {
6459
if (req) {
65-
const normalizedRequest = httpRequestToRequestData(req);
60+
const normalizedRequest = pagesRouterRequestToRequestData(req);
6661
scope.setSDKProcessingMetadata({ normalizedRequest });
6762
}
6863

‎packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
continueTrace,
55
debug,
66
getActiveSpan,
7-
httpRequestToRequestData,
87
isString,
98
isURLObjectRelative,
109
objectify,
@@ -17,6 +16,7 @@ import {
1716
} from '@sentry/core';
1817
import type { NextApiRequest } from 'next';
1918
import type { AugmentedNextApiResponse, NextApiHandler } from '../types';
19+
import { pagesRouterRequestToRequestData } from '../utils/pagesRouterRequestToRequestData';
2020
import { flushSafelyWithTimeout, waitUntil } from '../utils/responseEnd';
2121
import { dropNextjsRootContext, escapeNextjsTracing } from '../utils/tracingUtils';
2222
import { HTTP_ROUTE, URL_FULL, URL_PATH } from '@sentry/conventions/attributes';
@@ -77,7 +77,7 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz
7777
},
7878
() => {
7979
const reqMethod = `${(req.method || 'GET').toUpperCase()} `;
80-
const normalizedRequest = httpRequestToRequestData(req);
80+
const normalizedRequest = pagesRouterRequestToRequestData(req);
8181

8282
isolationScope.setSDKProcessingMetadata({ normalizedRequest });
8383
isolationScope.setTransactionName(`${reqMethod}${parameterizedRoute}`);
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import type { RequestEventData } from '@sentry/core';
2+
import { httpRequestToRequestData, parseUrl } from '@sentry/core';
3+
import type { IncomingMessage } from 'http';
4+
5+
const NEXT_REQUEST_META = Symbol.for('NextInternalRequestMeta');
6+
7+
type RequestWithNextMeta = IncomingMessage & {
8+
[NEXT_REQUEST_META]?: { initURL?: unknown };
9+
};
10+
11+
/**
12+
* Converts a Pages Router request into request data for events.
13+
*
14+
* Next.js strips `basePath` from `req.url` before running pages and API routes, but keeps the URL as it was originally
15+
* requested in its internal request meta (`initURL`). We use that one so the reported URL matches what was requested.
16+
*/
17+
export function pagesRouterRequestToRequestData(req: RequestWithNextMeta): RequestEventData {
18+
const requestData = httpRequestToRequestData(req);
19+
20+
const initUrl = req[NEXT_REQUEST_META]?.initURL;
21+
// `initURL` can be absolute, but its origin is built from the Next.js server's own hostname and port rather than the
22+
// request headers, so we only keep path and query and let the headers decide the origin like everywhere else.
23+
const originalUrl = typeof initUrl === 'string' ? parseUrl(initUrl).relative : undefined;
24+
if (!originalUrl || originalUrl === req.url) {
25+
return requestData;
26+
}
27+
28+
const { url, query_string } = httpRequestToRequestData({
29+
url: originalUrl,
30+
headers: req.headers,
31+
socket: req.socket,
32+
});
33+
34+
return { ...requestData, url, query_string };
35+
}

‎packages/nextjs/src/common/utils/wrapperUtils.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ import {
66
getIsolationScope,
77
getRootSpan,
88
getTraceData,
9-
httpRequestToRequestData,
109
isThenable,
1110
} from '@sentry/core';
1211
import type { IncomingMessage, ServerResponse } from 'http';
1312
import { TRANSACTION_ATTR_SENTRY_ROUTE_BACKFILL } from '../span-attributes-with-logic-attached';
13+
import { pagesRouterRequestToRequestData } from './pagesRouterRequestToRequestData';
1414

1515
/**
1616
* Wraps a function that potentially throws. If it does, the error is passed to `captureException` and rethrown.
@@ -68,7 +68,7 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
6868
this: unknown,
6969
...args: Parameters<F>
7070
): Promise<{ data: ReturnType<F>; sentryTrace?: string; baggage?: string }> {
71-
const normalizedRequest = httpRequestToRequestData(req);
71+
const normalizedRequest = pagesRouterRequestToRequestData(req);
7272
getCurrentScope().setTransactionName(`${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`);
7373
getIsolationScope().setSDKProcessingMetadata({ normalizedRequest });
7474

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import type { IncomingMessage } from 'http';
2+
import { describe, expect, it } from 'vitest';
3+
import { pagesRouterRequestToRequestData } from '../../../src/common/utils/pagesRouterRequestToRequestData';
4+
5+
function createRequest(url: string, initURL?: unknown): IncomingMessage {
6+
const req = {
7+
method: 'GET',
8+
url,
9+
headers: { host: 'example.com' },
10+
socket: {},
11+
} as unknown as IncomingMessage;
12+
13+
if (initURL !== undefined) {
14+
(req as unknown as Record<symbol, unknown>)[Symbol.for('NextInternalRequestMeta')] = { initURL };
15+
}
16+
17+
return req;
18+
}
19+
20+
describe('pagesRouterRequestToRequestData', () => {
21+
it('uses `req.url` when there is no Next.js request meta', () => {
22+
expect(pagesRouterRequestToRequestData(createRequest('/foo/bar?q=1'))).toMatchObject({
23+
url: 'http://example.com/foo/bar?q=1',
24+
query_string: 'q=1',
25+
method: 'GET',
26+
});
27+
});
28+
29+
it('restores the basePath from a relative `initURL`', () => {
30+
expect(pagesRouterRequestToRequestData(createRequest('/foo/bar?q=1', '/base/foo/bar?q=1'))).toMatchObject({
31+
url: 'http://example.com/base/foo/bar?q=1',
32+
query_string: 'q=1',
33+
method: 'GET',
34+
});
35+
});
36+
37+
it('restores the basePath for the basePath root', () => {
38+
expect(pagesRouterRequestToRequestData(createRequest('/', '/base'))).toMatchObject({
39+
url: 'http://example.com/base',
40+
});
41+
});
42+
43+
it('takes only path and query from an absolute `initURL` and keeps the origin from the request headers', () => {
44+
expect(
45+
pagesRouterRequestToRequestData(createRequest('/foo/bar', 'http://localhost:3000/base/foo/bar')),
46+
).toMatchObject({
47+
url: 'http://example.com/base/foo/bar',
48+
});
49+
});
50+
51+
it('ignores a non-string `initURL`', () => {
52+
expect(pagesRouterRequestToRequestData(createRequest('/foo/bar', 123))).toMatchObject({
53+
url: 'http://example.com/foo/bar',
54+
});
55+
});
56+
});

0 commit comments

Comments
 (0)