fix(core): Decode filename and module stack frame properties in Node stack parser (#14544)

In ESM, the Node stack trace filenames are URL-encoded. 

This patch:
- decodes the `filename` stack frame property
- decodes the `module` property (which is derived from the raw filename)
- adds a bunch of unit tests for the module name extraction logic
(general tests as well as for encoded file names)
- adds unit and integration tests (ESM and CJS) for file names with
spaces

Author: Lms24

dev-packages/node-integration-tests/suites/contextLines/instrument.mjs

@@ -0,0 +1,9 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  autoSessionTracking: false,
+  transport: loggingTransport,
+});

dev-packages/node-integration-tests/suites/contextLines/scenario with space.cjs

@@ -0,0 +1,13 @@
+const Sentry = require('@sentry/node');
+const { loggingTransport } = require('@sentry-internal/node-integration-tests');
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  autoSessionTracking: false,
+  transport: loggingTransport,
+});
+
+Sentry.captureException(new Error('Test Error'));
+
+// some more post context

dev-packages/node-integration-tests/suites/contextLines/scenario with space.mjs

@@ -0,0 +1,5 @@
+import * as Sentry from '@sentry/node';
+
+Sentry.captureException(new Error('Test Error'));
+
+// some more post context

dev-packages/node-integration-tests/suites/contextLines/test.ts

@@ -0,0 +1,83 @@
+import { join } from 'path';
+import { conditionalTest } from '../../utils';
+import { createRunner } from '../../utils/runner';
+
+conditionalTest({ min: 18 })('ContextLines integration in ESM', () => {
+  test('reads encoded context lines from filenames with spaces', done => {
+    expect.assertions(1);
+    const instrumentPath = join(__dirname, 'instrument.mjs');
+
+    createRunner(__dirname, 'scenario with space.mjs')
+      .withFlags('--import', instrumentPath)
+      .expect({
+        event: {
+          exception: {
+            values: [
+              {
+                value: 'Test Error',
+                stacktrace: {
+                  frames: expect.arrayContaining([
+                    {
+                      filename: expect.stringMatching(/\/scenario with space.mjs$/),
+                      context_line: "Sentry.captureException(new Error('Test Error'));",
+                      pre_context: ["import * as Sentry from '@sentry/node';", ''],
+                      post_context: ['', '// some more post context'],
+                      colno: 25,
+                      lineno: 3,
+                      function: '?',
+                      in_app: true,
+                      module: 'scenario with space',
+                    },
+                  ]),
+                },
+              },
+            ],
+          },
+        },
+      })
+      .start(done);
+  });
+});
+
+describe('ContextLines integration in CJS', () => {
+  test('reads context lines from filenames with spaces', done => {
+    expect.assertions(1);
+
+    createRunner(__dirname, 'scenario with space.cjs')
+      .expect({
+        event: {
+          exception: {
+            values: [
+              {
+                value: 'Test Error',
+                stacktrace: {
+                  frames: expect.arrayContaining([
+                    {
+                      filename: expect.stringMatching(/\/scenario with space.cjs$/),
+                      context_line: "Sentry.captureException(new Error('Test Error'));",
+                      pre_context: [
+                        'Sentry.init({',
+                        "  dsn: 'https://[email protected]/1337',",
+                        "  release: '1.0',",
+                        '  autoSessionTracking: false,',
+                        '  transport: loggingTransport,',
+                        '});',
+                        '',
+                      ],
+                      post_context: ['', '// some more post context'],
+                      colno: 25,
+                      lineno: 11,
+                      function: 'Object.?',
+                      in_app: true,
+                      module: 'scenario with space',
+                    },
+                  ]),
+                },
+              },
+            ],
+          },
+        },
+      })
+      .start(done);
+  });
+});

packages/core/src/utils-hoist/node-stack-trace.ts

@@ -113,7 +113,7 @@ export function node(getModule?: GetModuleFn): StackLineParserFn {
       }
 
       return {
-        filename,
+        filename: filename ? decodeURI(filename) : undefined,
         module: getModule ? getModule(filename) : undefined,
         function: functionName,
         lineno: _parseIntOrUndefined(lineMatch[3]),

packages/core/test/utils-hoist/stacktrace.test.ts

@@ -319,4 +319,64 @@ describe('node', () => {
     const result = node(line);
     expect(result?.in_app).toBe(true);
   });
+
+  it('parses frame filename paths with spaces and characters in file name', () => {
+    const input = 'at myObject.myMethod (/path/to/file with space(1).js:10:5)';
+
+    const expectedOutput = {
+      filename: '/path/to/file with space(1).js',
+      module: undefined,
+      function: 'myObject.myMethod',
+      lineno: 10,
+      colno: 5,
+      in_app: true,
+    };
+
+    expect(node(input)).toEqual(expectedOutput);
+  });
+
+  it('parses frame filename paths with spaces and characters in file path', () => {
+    const input = 'at myObject.myMethod (/path with space(1)/to/file.js:10:5)';
+
+    const expectedOutput = {
+      filename: '/path with space(1)/to/file.js',
+      module: undefined,
+      function: 'myObject.myMethod',
+      lineno: 10,
+      colno: 5,
+      in_app: true,
+    };
+
+    expect(node(input)).toEqual(expectedOutput);
+  });
+
+  it('parses encoded frame filename paths with spaces and characters in file name', () => {
+    const input = 'at myObject.myMethod (/path/to/file%20with%20space(1).js:10:5)';
+
+    const expectedOutput = {
+      filename: '/path/to/file with space(1).js',
+      module: undefined,
+      function: 'myObject.myMethod',
+      lineno: 10,
+      colno: 5,
+      in_app: true,
+    };
+
+    expect(node(input)).toEqual(expectedOutput);
+  });
+
+  it('parses encoded frame filename paths with spaces and characters in file path', () => {
+    const input = 'at myObject.myMethod (/path%20with%20space(1)/to/file.js:10:5)';
+
+    const expectedOutput = {
+      filename: '/path with space(1)/to/file.js',
+      module: undefined,
+      function: 'myObject.myMethod',
+      lineno: 10,
+      colno: 5,
+      in_app: true,
+    };
+
+    expect(node(input)).toEqual(expectedOutput);
+  });
 });

packages/node/src/utils/module.ts

@@ -29,29 +29,27 @@ export function createGetModuleFromFilename(
       file = file.slice(0, ext.length * -1);
     }
 
+    // The file name might be URI-encoded which we want to decode to
+    // the original file name.
+    const decodedFile = decodeURIComponent(file);
+
     if (!dir) {
       // No dirname whatsoever
       dir = '.';
     }
 
     const n = dir.lastIndexOf('/node_modules');
     if (n > -1) {
-      return `${dir.slice(n + 14).replace(/\//g, '.')}:${file}`;
+      return `${dir.slice(n + 14).replace(/\//g, '.')}:${decodedFile}`;
     }
 
     // Let's see if it's a part of the main module
     // To be a part of main module, it has to share the same base
     if (dir.startsWith(normalizedBase)) {
-      let moduleName = dir.slice(normalizedBase.length + 1).replace(/\//g, '.');
-
-      if (moduleName) {
-        moduleName += ':';
-      }
-      moduleName += file;
-
-      return moduleName;
+      const moduleName = dir.slice(normalizedBase.length + 1).replace(/\//g, '.');
+      return moduleName ? `${moduleName}:${decodedFile}` : decodedFile;
     }
 
-    return file;
+    return decodedFile;
   };
 }

packages/node/test/utils/module.test.ts

@@ -0,0 +1,46 @@
+import { createGetModuleFromFilename } from '../../src';
+
+describe('createGetModuleFromFilename', () => {
+  it.each([
+    ['/path/to/file.js', 'file'],
+    ['/path/to/file.mjs', 'file'],
+    ['/path/to/file.cjs', 'file'],
+    ['file.js', 'file'],
+  ])('returns the module name from a filename %s', (filename, expected) => {
+    const getModule = createGetModuleFromFilename();
+    expect(getModule(filename)).toBe(expected);
+  });
+
+  it('applies the given base path', () => {
+    const getModule = createGetModuleFromFilename('/path/to/base');
+    expect(getModule('/path/to/base/file.js')).toBe('file');
+  });
+
+  it('decodes URI-encoded file names', () => {
+    const getModule = createGetModuleFromFilename();
+    expect(getModule('/path%20with%space/file%20with%20spaces(1).js')).toBe('file with spaces(1)');
+  });
+
+  it('returns undefined if no filename is provided', () => {
+    const getModule = createGetModuleFromFilename();
+    expect(getModule(undefined)).toBeUndefined();
+  });
+
+  it.each([
+    ['/path/to/base/node_modules/@sentry/test/file.js', '@sentry.test:file'],
+    ['/path/to/base/node_modules/somePkg/file.js', 'somePkg:file'],
+  ])('handles node_modules file paths %s', (filename, expected) => {
+    const getModule = createGetModuleFromFilename();
+    expect(getModule(filename)).toBe(expected);
+  });
+
+  it('handles windows paths with passed basePath and node_modules', () => {
+    const getModule = createGetModuleFromFilename('C:\\path\\to\\base', true);
+    expect(getModule('C:\\path\\to\\base\\node_modules\\somePkg\\file.js')).toBe('somePkg:file');
+  });
+
+  it('handles windows paths with default basePath', () => {
+    const getModule = createGetModuleFromFilename(undefined, true);
+    expect(getModule('C:\\path\\to\\base\\somePkg\\file.js')).toBe('file');
+  });
+});