From 8a1321ad8d7fa2d34506570214024ee755f7ef55 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 5 Jun 2024 11:51:19 +0000 Subject: [PATCH 1/6] fix(nextjs): Fix parenthesis parsing logic for chromium --- .../test/unit/tracekit/chromium.test.ts | 17 +++++++++ .../test/unit/tracekit/firefox.test.ts | 35 +++++++++++++++++++ .../browser/test/unit/tracekit/safari.test.ts | 35 +++++++++++++++++++ 3 files changed, 87 insertions(+) diff --git a/packages/browser/test/unit/tracekit/chromium.test.ts b/packages/browser/test/unit/tracekit/chromium.test.ts index 56b5844711e7..c691fad6490e 100644 --- a/packages/browser/test/unit/tracekit/chromium.test.ts +++ b/packages/browser/test/unit/tracekit/chromium.test.ts @@ -1,6 +1,15 @@ import { exceptionFromError } from '../../../src/eventbuilder'; import { defaultStackParser as parser } from '../../../src/stack-parsers'; +const a = + 'Error\n' + + ' at onClick (http://localhost:3002/(group)/script.js:1:644)\n' + + ' at a (http://localhost:3002/[param]/script.js:1:644)\n' + + ' at b (http://localhost:3002/[param]/(group)/script.js:1:644)\n' + + ' at http://localhost:3002/[param]/script.js:1:644\n' + + ' at http://localhost:3002/[param]/(group)/script.js:1:644\n' + + ' at http://localhost:3002/(group)/script.js:1:644'; + describe('Tracekit - Chrome Tests', () => { it('should parse Chrome error with no location', () => { const NO_LOCATION = { message: 'foo', name: 'bar', stack: 'error\n at Array.forEach (native)' }; @@ -579,6 +588,7 @@ describe('Tracekit - Chrome Tests', () => { name: 'Error', stack: `Error: bad at something (http://localhost:5000/(some)/(thing)/index.html:20:16) + at http://localhost:5000//(group)/[route]/script.js:1:126 at more (http://localhost:5000/(some)/(thing)/index.html:25:7)`, }; @@ -596,6 +606,13 @@ describe('Tracekit - Chrome Tests', () => { colno: 7, in_app: true, }, + { + filename: 'http://localhost:5000/(group)/[route]/script.js', + function: '?', + lineno: 1, + colno: 126, + in_app: true, + }, { filename: 'http://localhost:5000/(some)/(thing)/index.html', function: 'something', diff --git a/packages/browser/test/unit/tracekit/firefox.test.ts b/packages/browser/test/unit/tracekit/firefox.test.ts index f75dd7ccf010..5e05930f8078 100644 --- a/packages/browser/test/unit/tracekit/firefox.test.ts +++ b/packages/browser/test/unit/tracekit/firefox.test.ts @@ -311,6 +311,41 @@ describe('Tracekit - Firefox Tests', () => { }); }); + it('should correctly parse parentheses', () => { + const PARENTHESIS_FRAME_EXCEPTION = { + message: 'aha', + name: 'Error', + stack: + 'onClick@http://localhost:3002/_next/static/chunks/app/(group)/[route]/script.js:1:644\n' + + '@http://localhost:3002/_next/static/chunks/app/(group)/[route]/script.js:1:126', + }; + + const stacktrace = exceptionFromError(parser, PARENTHESIS_FRAME_EXCEPTION); + + expect(stacktrace).toEqual({ + value: 'aha', + type: 'Error', + stacktrace: { + frames: [ + { + colno: 126, + filename: 'http://localhost:3002/_next/static/chunks/app/(group)/[route]/script.js', + function: '?', + in_app: true, + lineno: 1, + }, + { + colno: 644, + filename: 'http://localhost:3002/_next/static/chunks/app/(group)/[route]/script.js', + function: 'onClick', + in_app: true, + lineno: 1, + }, + ], + }, + }); + }); + it('should parse Firefox errors with `file` inside an identifier', () => { const FIREFOX_FILE_IN_IDENTIFIER = { stack: diff --git a/packages/browser/test/unit/tracekit/safari.test.ts b/packages/browser/test/unit/tracekit/safari.test.ts index 657ffc7daecc..470ed1d7b8dc 100644 --- a/packages/browser/test/unit/tracekit/safari.test.ts +++ b/packages/browser/test/unit/tracekit/safari.test.ts @@ -320,4 +320,39 @@ describe('Tracekit - Safari Tests', () => { }, }); }); + + it('should correctly parse parentheses', () => { + const PARENTHESIS_FRAME_EXCEPTION = { + message: 'aha', + name: 'Error', + stack: + '@http://localhost:3000/(group)/[route]/script.js:1:131\n' + + 'global code@http://localhost:3000/(group)/[route]/script.js:1:334', + }; + + const ex = exceptionFromError(parser, PARENTHESIS_FRAME_EXCEPTION); + + expect(ex).toEqual({ + value: 'aha', + type: 'Error', + stacktrace: { + frames: [ + { + colno: 334, + filename: 'http://localhost:3000/(group)/[route]/script.js', + function: 'global code', + in_app: true, + lineno: 1, + }, + { + colno: 131, + filename: 'http://localhost:3000/(group)/[route]/script.js', + function: '?', + in_app: true, + lineno: 1, + }, + ], + }, + }); + }); }); From 91bdf9402bb39ef396cb2264d0163228af036676 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 5 Jun 2024 11:53:02 +0000 Subject: [PATCH 2/6] woops --- packages/browser/test/unit/tracekit/chromium.test.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/browser/test/unit/tracekit/chromium.test.ts b/packages/browser/test/unit/tracekit/chromium.test.ts index c691fad6490e..fe5ca5edc221 100644 --- a/packages/browser/test/unit/tracekit/chromium.test.ts +++ b/packages/browser/test/unit/tracekit/chromium.test.ts @@ -1,15 +1,6 @@ import { exceptionFromError } from '../../../src/eventbuilder'; import { defaultStackParser as parser } from '../../../src/stack-parsers'; -const a = - 'Error\n' + - ' at onClick (http://localhost:3002/(group)/script.js:1:644)\n' + - ' at a (http://localhost:3002/[param]/script.js:1:644)\n' + - ' at b (http://localhost:3002/[param]/(group)/script.js:1:644)\n' + - ' at http://localhost:3002/[param]/script.js:1:644\n' + - ' at http://localhost:3002/[param]/(group)/script.js:1:644\n' + - ' at http://localhost:3002/(group)/script.js:1:644'; - describe('Tracekit - Chrome Tests', () => { it('should parse Chrome error with no location', () => { const NO_LOCATION = { message: 'foo', name: 'bar', stack: 'error\n at Array.forEach (native)' }; From 1933b22fcc3b2fd5f0958ffeda473a8cc7a1d52e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 5 Jun 2024 12:27:28 +0000 Subject: [PATCH 3/6] typo --- packages/browser/test/unit/tracekit/chromium.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/test/unit/tracekit/chromium.test.ts b/packages/browser/test/unit/tracekit/chromium.test.ts index fe5ca5edc221..790f36e0ddc3 100644 --- a/packages/browser/test/unit/tracekit/chromium.test.ts +++ b/packages/browser/test/unit/tracekit/chromium.test.ts @@ -579,7 +579,7 @@ describe('Tracekit - Chrome Tests', () => { name: 'Error', stack: `Error: bad at something (http://localhost:5000/(some)/(thing)/index.html:20:16) - at http://localhost:5000//(group)/[route]/script.js:1:126 + at http://localhost:5000/(group)/[route]/script.js:1:126 at more (http://localhost:5000/(some)/(thing)/index.html:25:7)`, }; From 18b5053fd0750b413e39ef9557a2c74f23b622b4 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 5 Jun 2024 14:47:58 +0200 Subject: [PATCH 4/6] Fix parentheses parsing when there's no function name --- packages/browser/src/stack-parsers.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/browser/src/stack-parsers.ts b/packages/browser/src/stack-parsers.ts index a5a303cd1375..3a14204319e1 100644 --- a/packages/browser/src/stack-parsers.ts +++ b/packages/browser/src/stack-parsers.ts @@ -51,6 +51,7 @@ function createFrame(filename: string, func: string, lineno?: number, colno?: nu } // Chromium based browsers: Chrome, Brave, new Opera, new Edge +const chromeRegexNoFnName = /^\s*at (\S+?)(?::(\d+))(?::(\d+))\s*$/i; const chromeRegex = /^\s*at (?:(.+?\)(?: \[.+\])?|.*?) ?\((?:address at )?)?(?:async )?((?:|[-a-z]+:|.*bundle|\/)?.*?)(?::(\d+))?(?::(\d+))?\)?\s*$/i; const chromeEvalRegex = /\((\S*)(?::(\d+))(?::(\d+))\)/; @@ -58,6 +59,14 @@ const chromeEvalRegex = /\((\S*)(?::(\d+))(?::(\d+))\)/; // We cannot call this variable `chrome` because it can conflict with global `chrome` variable in certain environments // See: https://github.com/getsentry/sentry-javascript/issues/6880 const chromeStackParserFn: StackLineParserFn = line => { + // If the stack line has no function name, we need to parse it differently + const noFnParts = chromeRegexNoFnName.exec(line); + + if (noFnParts) { + const [, filename, line, col] = noFnParts; + return createFrame(filename, UNKNOWN_FUNCTION, +line, +col); + } + const parts = chromeRegex.exec(line); if (parts) { From e8a340e7b23c8bfd9abae86e64bf2690dbf9cbb4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 5 Jun 2024 13:18:41 +0000 Subject: [PATCH 5/6] bump size limit --- .size-limit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.js b/.size-limit.js index 0784ca597a7c..a8969c330838 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -22,7 +22,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration'), gzip: true, - limit: '70 KB', + limit: '71 KB', }, { name: '@sentry/browser (incl. Tracing, Replay) - with treeshaking flags', From b5851c7c723ee9f1f573e10eb1d7ddec2f4723c6 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 6 Jun 2024 08:19:40 +0000 Subject: [PATCH 6/6] Add comments --- packages/browser/src/stack-parsers.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/browser/src/stack-parsers.ts b/packages/browser/src/stack-parsers.ts index 3a14204319e1..033d6ee3411b 100644 --- a/packages/browser/src/stack-parsers.ts +++ b/packages/browser/src/stack-parsers.ts @@ -50,12 +50,18 @@ function createFrame(filename: string, func: string, lineno?: number, colno?: nu return frame; } -// Chromium based browsers: Chrome, Brave, new Opera, new Edge +// This regex matches frames that have no function name (ie. are at the top level of a module). +// For example "at http://localhost:5000//script.js:1:126" +// Frames _with_ function names usually look as follows: "at commitLayoutEffects (react-dom.development.js:23426:1)" const chromeRegexNoFnName = /^\s*at (\S+?)(?::(\d+))(?::(\d+))\s*$/i; + +// This regex matches all the frames that have a function name. const chromeRegex = /^\s*at (?:(.+?\)(?: \[.+\])?|.*?) ?\((?:address at )?)?(?:async )?((?:|[-a-z]+:|.*bundle|\/)?.*?)(?::(\d+))?(?::(\d+))?\)?\s*$/i; + const chromeEvalRegex = /\((\S*)(?::(\d+))(?::(\d+))\)/; +// Chromium based browsers: Chrome, Brave, new Opera, new Edge // We cannot call this variable `chrome` because it can conflict with global `chrome` variable in certain environments // See: https://github.com/getsentry/sentry-javascript/issues/6880 const chromeStackParserFn: StackLineParserFn = line => {