feat(core): Add default behaviour for rewriteFramesIntegration in browser (#11535)

Author: lforst

packages/core/src/integrations/rewriteframes.ts

@@ -1,45 +1,61 @@
-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;
 
 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;
+  };
+}

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<typeof rewriteFramesIntegration>;
 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
+      });
+    });
+  });
 });