From d205f8de1b07b7c5e724a5c7c8ab243112e59190 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 8 Feb 2023 15:28:45 +0000 Subject: [PATCH 1/3] feat(nextjs): Improve client stack traces --- packages/nextjs/src/client/index.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index 8537c15cb963..f38db4e2296a 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -69,6 +69,20 @@ function addClientIntegrations(options: BrowserOptions): void { // Filename wasn't a properly formed URL, so there's nothing we can do } + if (frame.filename && frame.filename.startsWith('app:///_next')) { + // We need to URI-decode the filename because Next.js has wildcard routes like "/users/[id].js" which show up as "/users/%5id%5.js" in Error stacktraces. + // The corresponding sources that Next.js generates have proper brackets so we also need proper brackets in the frame so that source map resolving works. + frame.filename = decodeURI(frame.filename); + } + + if ( + frame.filename && + frame.filename.match(/^app:\/\/\/_next\/static\/chunks\/(main|main-app|polyfills|webpack)-[0-9a-f]{16}\.js$/) + ) { + // We don't care about these frames. It's Next.js internal code. + frame.in_app = false; + } + return frame; }, }); From 781979c471505c500d67f75396f9f5785c7f63d2 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 8 Feb 2023 17:28:15 +0000 Subject: [PATCH 2/3] Add test --- .../test/integration/pages/[id]/errorClick.tsx | 11 +++++++++++ .../integration/test/client/errorClick.test.ts | 15 ++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 packages/nextjs/test/integration/pages/[id]/errorClick.tsx diff --git a/packages/nextjs/test/integration/pages/[id]/errorClick.tsx b/packages/nextjs/test/integration/pages/[id]/errorClick.tsx new file mode 100644 index 000000000000..df55a4b32967 --- /dev/null +++ b/packages/nextjs/test/integration/pages/[id]/errorClick.tsx @@ -0,0 +1,11 @@ +const ButtonPage = (): JSX.Element => ( + +); + +export default ButtonPage; diff --git a/packages/nextjs/test/integration/test/client/errorClick.test.ts b/packages/nextjs/test/integration/test/client/errorClick.test.ts index 78baee9dad8a..a18593b7c74b 100644 --- a/packages/nextjs/test/integration/test/client/errorClick.test.ts +++ b/packages/nextjs/test/integration/test/client/errorClick.test.ts @@ -5,7 +5,7 @@ import { Event } from '@sentry/types'; test('should capture error triggered on click', async ({ page }) => { await page.goto('/errorClick'); - const [_, events] = await Promise.all([ + const [, events] = await Promise.all([ page.click('button'), getMultipleSentryEnvelopeRequests(page, 1, { envelopeType: 'event' }), ]); @@ -15,3 +15,16 @@ test('should capture error triggered on click', async ({ page }) => { value: 'Sentry Frontend Error', }); }); + +test('should have a non-url-encoded top frame in route with parameter', async ({ page }) => { + await page.goto('/some-param/errorClick'); + + const [, events] = await Promise.all([ + page.click('button'), + getMultipleSentryEnvelopeRequests(page, 1, { envelopeType: 'event' }), + ]); + + const frames = events[0]?.exception?.values?.[0].stacktrace?.frames; + + expect(frames?.[frames.length - 1].filename).toMatch(/\/\[id\]\/errorClick-[a-f0-9]{20}\.js$/); +}); From bb005fe1a41015c69a17d97b43183f31ff1bd41a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Feb 2023 09:25:32 +0000 Subject: [PATCH 3/3] Fix/improve tests --- packages/nextjs/src/client/index.ts | 4 ++- .../test/client/errorClick.test.ts | 31 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index f38db4e2296a..c2d512b3ef00 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -77,7 +77,9 @@ function addClientIntegrations(options: BrowserOptions): void { if ( frame.filename && - frame.filename.match(/^app:\/\/\/_next\/static\/chunks\/(main|main-app|polyfills|webpack)-[0-9a-f]{16}\.js$/) + frame.filename.match( + /^app:\/\/\/_next\/static\/chunks\/(main-|main-app-|polyfills-|webpack-|framework-|framework\.)[0-9a-f]+\.js$/, + ) ) { // We don't care about these frames. It's Next.js internal code. frame.in_app = false; diff --git a/packages/nextjs/test/integration/test/client/errorClick.test.ts b/packages/nextjs/test/integration/test/client/errorClick.test.ts index a18593b7c74b..aa73e54ec4f7 100644 --- a/packages/nextjs/test/integration/test/client/errorClick.test.ts +++ b/packages/nextjs/test/integration/test/client/errorClick.test.ts @@ -26,5 +26,34 @@ test('should have a non-url-encoded top frame in route with parameter', async ({ const frames = events[0]?.exception?.values?.[0].stacktrace?.frames; - expect(frames?.[frames.length - 1].filename).toMatch(/\/\[id\]\/errorClick-[a-f0-9]{20}\.js$/); + expect(frames?.[frames.length - 1].filename).toMatch(/\/\[id\]\/errorClick-[a-f0-9]+\.js$/); +}); + +test('should mark nextjs internal frames as `in_app`: false', async ({ page }) => { + await page.goto('/some-param/errorClick'); + + const [, events] = await Promise.all([ + page.click('button'), + getMultipleSentryEnvelopeRequests(page, 1, { envelopeType: 'event' }), + ]); + + const frames = events[0]?.exception?.values?.[0].stacktrace?.frames; + + expect(frames).toContainEqual( + expect.objectContaining({ + filename: expect.stringMatching( + /^app:\/\/\/_next\/static\/chunks\/(main-|main-app-|polyfills-|webpack-|framework-|framework\.)[0-9a-f]+\.js$/, + ), + in_app: false, + }), + ); + + expect(frames).not.toContainEqual( + expect.objectContaining({ + filename: expect.stringMatching( + /^app:\/\/\/_next\/static\/chunks\/(main-|main-app-|polyfills-|webpack-|framework-|framework\.)[0-9a-f]+\.js$/, + ), + in_app: true, + }), + ); });