add more tests

Author: Lms24

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

@@ -6,5 +6,4 @@ Sentry.init({
   release: '1.0',
   autoSessionTracking: false,
   transport: loggingTransport,
-  debug: true,
 });

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/test.ts

@@ -2,7 +2,7 @@ import { join } from 'path';
 import { createRunner } from '../../utils/runner';
 
 describe('ContextLines integration', () => {
-  test('reads context lines from filenames with spaces', done => {
+  test('reads encoded context lines from filenames with spaces (ESM)', done => {
     expect.assertions(1);
     const instrumentPath = join(__dirname, 'instrument.mjs');
 
@@ -36,4 +36,45 @@ describe('ContextLines integration', () => {
       })
       .start(done);
   });
+
+  test('reads context lines from filenames with spaces (CJS)', 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/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

@@ -42,14 +42,8 @@ export function createGetModuleFromFilename(
     // 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}:${file}` : file;
     }
 
     return file;

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

@@ -0,0 +1,41 @@
+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('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', () => {
+    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');
+  });
+});