diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts index e33e3243db47..b326781ba5fd 100644 --- a/packages/nextjs/src/performance/client.ts +++ b/packages/nextjs/src/performance/client.ts @@ -1,5 +1,5 @@ import { Primitive, Transaction, TransactionContext } from '@sentry/types'; -import { fill, getGlobalObject } from '@sentry/utils'; +import { fill, getGlobalObject, stripUrlQueryAndFragment } from '@sentry/utils'; import { default as Router } from 'next/router'; const global = getGlobalObject(); @@ -10,8 +10,6 @@ const DEFAULT_TAGS = Object.freeze({ 'routing.instrumentation': 'next-router', }); -const QUERY_PARAM_REGEX = /\?(.*)/; - let activeTransaction: Transaction | undefined = undefined; let prevTransactionName: string | undefined = undefined; let startTransaction: StartTransactionCb | undefined = undefined; @@ -35,7 +33,7 @@ export function nextRouterInstrumentation( // route name. Setting the transaction name after the transaction is started could lead // to possible race conditions with the router, so this approach was taken. if (startTransactionOnPageLoad) { - prevTransactionName = Router.route !== null ? removeQueryParams(Router.route) : global.location.pathname; + prevTransactionName = Router.route !== null ? stripUrlQueryAndFragment(Router.route) : global.location.pathname; activeTransaction = startTransactionCb({ name: prevTransactionName, op: 'pageload', @@ -98,7 +96,7 @@ function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): Wrap if (prevTransactionName) { tags.from = prevTransactionName; } - prevTransactionName = removeQueryParams(url); + prevTransactionName = stripUrlQueryAndFragment(url); activeTransaction = startTransaction({ name: prevTransactionName, op: 'navigation', @@ -109,7 +107,3 @@ function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): Wrap }; return wrapper; } - -export function removeQueryParams(route: string): string { - return route.replace(QUERY_PARAM_REGEX, ''); -} diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts index d1effd5003ee..3514f436a010 100644 --- a/packages/nextjs/src/utils/instrumentServer.ts +++ b/packages/nextjs/src/utils/instrumentServer.ts @@ -1,7 +1,7 @@ import { deepReadDirSync } from '@sentry/node'; import { extractTraceparentData, getActiveTransaction, hasTracingEnabled } from '@sentry/tracing'; import { Event as SentryEvent } from '@sentry/types'; -import { fill, isString, logger } from '@sentry/utils'; +import { fill, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'; import * as domain from 'domain'; import * as http from 'http'; import { default as createNextServer } from 'next'; @@ -208,7 +208,7 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler { } // pull off query string, if any - const reqPath = req.url.split('?')[0]; + const reqPath = stripUrlQueryAndFragment(req.url); // requests for pages will only ever be GET requests, so don't bother to include the method in the transaction // 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 { { name: `${namePrefix}${reqPath}`, op: 'http.server', - metadata: { requestPath: req.url.split('?')[0] }, + metadata: { requestPath: reqPath }, ...traceparentData, }, // extra context passed to the `tracesSampler` diff --git a/packages/nextjs/test/performance/client.test.ts b/packages/nextjs/test/performance/client.test.ts index 46ec3e60f051..ac632349275c 100644 --- a/packages/nextjs/test/performance/client.test.ts +++ b/packages/nextjs/test/performance/client.test.ts @@ -1,6 +1,6 @@ import { default as Router } from 'next/router'; -import { nextRouterInstrumentation, removeQueryParams } from '../../src/performance/client'; +import { nextRouterInstrumentation } from '../../src/performance/client'; let readyCalled = false; jest.mock('next/router', () => { @@ -100,18 +100,4 @@ describe('client', () => { }); }); }); - - describe('removeQueryParams()', () => { - it('removes query params from an url', () => { - const table: Table = [ - { in: '/posts/[id]/[comment]?name=ferret&color=purple', out: '/posts/[id]/[comment]' }, - { in: '/posts/[id]/[comment]?', out: '/posts/[id]/[comment]' }, - { in: '/about?', out: '/about' }, - ]; - - table.forEach(test => { - expect(removeQueryParams(test.in)).toEqual(test.out); - }); - }); - }); });