Skip to content

Commit 6afe381

Browse files
authored
ref(nextjs): Use common util to strip query params (#3598)
Use the stripUrlQueryAndFragment function from @sentry/utils instead of using our implementation.
1 parent b6f9dc6 commit 6afe381

File tree

3 files changed

+7
-27
lines changed

3 files changed

+7
-27
lines changed

packages/nextjs/src/performance/client.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Primitive, Transaction, TransactionContext } from '@sentry/types';
2-
import { fill, getGlobalObject } from '@sentry/utils';
2+
import { fill, getGlobalObject, stripUrlQueryAndFragment } from '@sentry/utils';
33
import { default as Router } from 'next/router';
44

55
const global = getGlobalObject<Window>();
@@ -10,8 +10,6 @@ const DEFAULT_TAGS = Object.freeze({
1010
'routing.instrumentation': 'next-router',
1111
});
1212

13-
const QUERY_PARAM_REGEX = /\?(.*)/;
14-
1513
let activeTransaction: Transaction | undefined = undefined;
1614
let prevTransactionName: string | undefined = undefined;
1715
let startTransaction: StartTransactionCb | undefined = undefined;
@@ -35,7 +33,7 @@ export function nextRouterInstrumentation(
3533
// route name. Setting the transaction name after the transaction is started could lead
3634
// to possible race conditions with the router, so this approach was taken.
3735
if (startTransactionOnPageLoad) {
38-
prevTransactionName = Router.route !== null ? removeQueryParams(Router.route) : global.location.pathname;
36+
prevTransactionName = Router.route !== null ? stripUrlQueryAndFragment(Router.route) : global.location.pathname;
3937
activeTransaction = startTransactionCb({
4038
name: prevTransactionName,
4139
op: 'pageload',
@@ -98,7 +96,7 @@ function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): Wrap
9896
if (prevTransactionName) {
9997
tags.from = prevTransactionName;
10098
}
101-
prevTransactionName = removeQueryParams(url);
99+
prevTransactionName = stripUrlQueryAndFragment(url);
102100
activeTransaction = startTransaction({
103101
name: prevTransactionName,
104102
op: 'navigation',
@@ -109,7 +107,3 @@ function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): Wrap
109107
};
110108
return wrapper;
111109
}
112-
113-
export function removeQueryParams(route: string): string {
114-
return route.replace(QUERY_PARAM_REGEX, '');
115-
}

packages/nextjs/src/utils/instrumentServer.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { deepReadDirSync } from '@sentry/node';
22
import { extractTraceparentData, getActiveTransaction, hasTracingEnabled } from '@sentry/tracing';
33
import { Event as SentryEvent } from '@sentry/types';
4-
import { fill, isString, logger } from '@sentry/utils';
4+
import { fill, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils';
55
import * as domain from 'domain';
66
import * as http from 'http';
77
import { default as createNextServer } from 'next';
@@ -208,7 +208,7 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
208208
}
209209

210210
// pull off query string, if any
211-
const reqPath = req.url.split('?')[0];
211+
const reqPath = stripUrlQueryAndFragment(req.url);
212212

213213
// requests for pages will only ever be GET requests, so don't bother to include the method in the transaction
214214
// name; requests to API routes could be GET, POST, PUT, etc, so do include it there
@@ -218,7 +218,7 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
218218
{
219219
name: `${namePrefix}${reqPath}`,
220220
op: 'http.server',
221-
metadata: { requestPath: req.url.split('?')[0] },
221+
metadata: { requestPath: reqPath },
222222
...traceparentData,
223223
},
224224
// extra context passed to the `tracesSampler`

packages/nextjs/test/performance/client.test.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { default as Router } from 'next/router';
22

3-
import { nextRouterInstrumentation, removeQueryParams } from '../../src/performance/client';
3+
import { nextRouterInstrumentation } from '../../src/performance/client';
44

55
let readyCalled = false;
66
jest.mock('next/router', () => {
@@ -100,18 +100,4 @@ describe('client', () => {
100100
});
101101
});
102102
});
103-
104-
describe('removeQueryParams()', () => {
105-
it('removes query params from an url', () => {
106-
const table: Table = [
107-
{ in: '/posts/[id]/[comment]?name=ferret&color=purple', out: '/posts/[id]/[comment]' },
108-
{ in: '/posts/[id]/[comment]?', out: '/posts/[id]/[comment]' },
109-
{ in: '/about?', out: '/about' },
110-
];
111-
112-
table.forEach(test => {
113-
expect(removeQueryParams(test.in)).toEqual(test.out);
114-
});
115-
});
116-
});
117103
});

0 commit comments

Comments
 (0)