From 1eac4cd4fea720af5e0826f1ba4a027da9c36b5b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 11 Apr 2024 13:13:09 +0200 Subject: [PATCH] feat(core): Add default behaviour for `rewriteFramesIntegration` in browser (#11535) --- .../core/src/integrations/rewriteframes.ts | 120 +++++++++++++----- .../lib/integrations/rewriteframes.test.ts | 39 +++++- 2 files changed, 128 insertions(+), 31 deletions(-) diff --git a/packages/core/src/integrations/rewriteframes.ts b/packages/core/src/integrations/rewriteframes.ts index ee842e86942f..3b81d141cdad 100644 --- a/packages/core/src/integrations/rewriteframes.ts +++ b/packages/core/src/integrations/rewriteframes.ts @@ -1,5 +1,5 @@ -import type { Event, IntegrationFn, StackFrame, Stacktrace } from '@sentry/types'; -import { basename, relative } from '@sentry/utils'; +import type { Event, StackFrame, Stacktrace } from '@sentry/types'; +import { GLOBAL_OBJ, basename, relative } from '@sentry/utils'; import { defineIntegration } from '../integration'; type StackFrameIteratee = (frame: StackFrame) => StackFrame; @@ -7,39 +7,55 @@ type StackFrameIteratee = (frame: StackFrame) => StackFrame; const INTEGRATION_NAME = 'RewriteFrames'; interface RewriteFramesOptions { + /** + * Root path (the beginning of the path) that will be stripped from the frames' filename. + * + * This option has slightly different behaviour in the browser and on servers: + * - In the browser, the value you provide in `root` will be stripped from the beginning stack frames' paths (if the path started with the value). + * - On the server, the root value will only replace the beginning of stack frame filepaths, when the path is absolute. If no `root` value is provided and the path is absolute, the frame will be reduced to only the filename and the provided `prefix` option. + * + * Browser example: + * - Original frame: `'http://example.com/my/path/static/asset.js'` + * - `root: 'http://example.com/my/path'` + * - `assetPrefix: 'app://'` + * - Resulting frame: `'app:///static/asset.js'` + * + * Server example: + * - Original frame: `'/User/local/my/path/static/asset.js'` + * - `root: '/User/local/my/path'` + * - `assetPrefix: 'app://'` + * - Resulting frame: `'app:///static/asset.js'` + */ root?: string; + + /** + * A custom prefix that stack frames will be prepended with. + * + * Default: `'app://'` + * + * This option has slightly different behaviour in the browser and on servers: + * - In the browser, the value you provide in `prefix` will prefix the resulting filename when the value you provided in `root` was applied. Effectively replacing whatever `root` matched in the beginning of the frame with `prefix`. + * - On the server, the prefix is applied to all stackframes with absolute paths. On Windows, the drive identifier (e.g. "C://") is replaced with the prefix. + */ prefix?: string; + + /** + * Defines an iterator that is used to iterate through all of the stack frames for modification before being sent to Sentry. + * Setting this option will effectively disable both the `root` and the `prefix` options. + */ iteratee?: StackFrameIteratee; } -const _rewriteFramesIntegration = ((options: RewriteFramesOptions = {}) => { +/** + * Rewrite event frames paths. + */ +export const rewriteFramesIntegration = defineIntegration((options: RewriteFramesOptions = {}) => { const root = options.root; const prefix = options.prefix || 'app:///'; - const iteratee: StackFrameIteratee = - options.iteratee || - ((frame: StackFrame) => { - if (!frame.filename) { - return frame; - } - // Determine if this is a Windows frame by checking for a Windows-style prefix such as `C:\` - const isWindowsFrame = - /^[a-zA-Z]:\\/.test(frame.filename) || - // or the presence of a backslash without a forward slash (which are not allowed on Windows) - (frame.filename.includes('\\') && !frame.filename.includes('/')); - // Check if the frame filename begins with `/` - const startsWithSlash = /^\//.test(frame.filename); - if (isWindowsFrame || startsWithSlash) { - const filename = isWindowsFrame - ? frame.filename - .replace(/^[a-zA-Z]:/, '') // remove Windows-style prefix - .replace(/\\/g, '/') // replace all `\\` instances with `/` - : frame.filename; - const base = root ? relative(root, filename) : basename(filename); - frame.filename = `${prefix}${base}`; - } - return frame; - }); + const isBrowser = 'window' in GLOBAL_OBJ && GLOBAL_OBJ.window !== undefined; + + const iteratee: StackFrameIteratee = options.iteratee || generateIteratee({ isBrowser, root, prefix }); /** Process an exception event. */ function _processExceptionsEvent(event: Event): Event { @@ -81,9 +97,53 @@ const _rewriteFramesIntegration = ((options: RewriteFramesOptions = {}) => { return processedEvent; }, }; -}) satisfies IntegrationFn; +}); /** - * Rewrite event frames paths. + * Exported only for tests. */ -export const rewriteFramesIntegration = defineIntegration(_rewriteFramesIntegration); +export function generateIteratee({ + isBrowser, + root, + prefix, +}: { + isBrowser: boolean; + root?: string; + prefix: string; +}): StackFrameIteratee { + return (frame: StackFrame) => { + if (!frame.filename) { + return frame; + } + + // Determine if this is a Windows frame by checking for a Windows-style prefix such as `C:\` + const isWindowsFrame = + /^[a-zA-Z]:\\/.test(frame.filename) || + // or the presence of a backslash without a forward slash (which are not allowed on Windows) + (frame.filename.includes('\\') && !frame.filename.includes('/')); + + // Check if the frame filename begins with `/` + const startsWithSlash = /^\//.test(frame.filename); + + if (isBrowser) { + if (root) { + const oldFilename = frame.filename; + if (oldFilename.indexOf(root) === 0) { + frame.filename = oldFilename.replace(root, prefix); + } + } + } else { + if (isWindowsFrame || startsWithSlash) { + const filename = isWindowsFrame + ? frame.filename + .replace(/^[a-zA-Z]:/, '') // remove Windows-style prefix + .replace(/\\/g, '/') // replace all `\\` instances with `/` + : frame.filename; + const base = root ? relative(root, filename) : basename(filename); + frame.filename = `${prefix}${base}`; + } + } + + return frame; + }; +} diff --git a/packages/core/test/lib/integrations/rewriteframes.test.ts b/packages/core/test/lib/integrations/rewriteframes.test.ts index 17d6ed4b9c5b..4c30de3a42a6 100644 --- a/packages/core/test/lib/integrations/rewriteframes.test.ts +++ b/packages/core/test/lib/integrations/rewriteframes.test.ts @@ -1,6 +1,6 @@ import type { Event, StackFrame } from '@sentry/types'; -import { rewriteFramesIntegration } from '../../../src/integrations/rewriteframes'; +import { generateIteratee, rewriteFramesIntegration } from '../../../src/integrations/rewriteframes'; let rewriteFrames: ReturnType; let exceptionEvent: Event; @@ -11,6 +11,17 @@ let windowsExceptionEventWithoutPrefix: Event; let windowsExceptionEventWithBackslashPrefix: Event; let multipleStacktracesEvent: Event; +const originalWindow = global.window; + +beforeAll(() => { + // @ts-expect-error We need to do this because the integration has different behaviour on the browser and on the client + global.window = undefined; +}); + +afterAll(() => { + global.window = originalWindow; +}); + describe('RewriteFrames', () => { beforeEach(() => { exceptionEvent = { @@ -298,4 +309,30 @@ describe('RewriteFrames', () => { expect(rewriteFrames.processEvent?.(brokenEvent, {}, {} as any)).toEqual(brokenEvent); }); }); + + describe('generateIteratee()', () => { + describe('on the browser', () => { + it('should replace the `root` value in the filename with the `assetPrefix` value', () => { + const iteratee = generateIteratee({ + isBrowser: true, + prefix: 'my-prefix://', + root: 'http://example.com/my/path', + }); + + const result = iteratee({ filename: 'http://example.com/my/path/static/asset.js' }); + expect(result.filename).toBe('my-prefix:///static/asset.js'); + }); + + it('should replace not the `root` value in the filename with the `assetPrefix` value, if the root value is not at the beginning of the frame', () => { + const iteratee = generateIteratee({ + isBrowser: true, + prefix: 'my-prefix://', + root: '/my/path', + }); + + const result = iteratee({ filename: 'http://example.com/my/path/static/asset.js' }); + expect(result.filename).toBe('http://example.com/my/path/static/asset.js'); // unchanged + }); + }); + }); });