Skip to content

Commit

Permalink
feat(node): Make getModuleFromFilename compatible with ESM (#10061)
Browse files Browse the repository at this point in the history
This removes usage of `require.main.filename` to bring better ESM
compatability.

An extra bonus in that now `createGetModuleFromFilename` is ESM
compatible, we can use it in the Anr worker when `appRootPath` is
supplied (ie. Electron only for now).

### ⚠️ Fingerprinting 
This may change `module` fingerprinting for ESM since previously when
`require.main.filename` was not available, `process.cwd()` was used.
  • Loading branch information
timfish committed Jan 9, 2024
1 parent 5623fd8 commit eff57fa
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 74 deletions.
2 changes: 2 additions & 0 deletions packages/node-experimental/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ export {
extractRequestData,
// eslint-disable-next-line deprecation/deprecation
deepReadDirSync,
// eslint-disable-next-line deprecation/deprecation
getModuleFromFilename,
createGetModuleFromFilename,
close,
createTransport,
// eslint-disable-next-line deprecation/deprecation
Expand Down
9 changes: 8 additions & 1 deletion packages/node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,14 @@ export { defaultIntegrations, init, defaultStackParser, getSentryRelease } from
export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from '@sentry/utils';
// eslint-disable-next-line deprecation/deprecation
export { deepReadDirSync } from './utils';
export { getModuleFromFilename } from './module';

import { createGetModuleFromFilename } from './module';
/**
* @deprecated use `createGetModuleFromFilename` instead.
*/
export const getModuleFromFilename = createGetModuleFromFilename();
export { createGetModuleFromFilename };

// eslint-disable-next-line deprecation/deprecation
export { enableAnrDetection } from './integrations/anr/legacy';

Expand Down
5 changes: 4 additions & 1 deletion packages/node/src/integrations/anr/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
} from '@sentry/utils';
import { Session as InspectorSession } from 'inspector';
import { parentPort, workerData } from 'worker_threads';

import { createGetModuleFromFilename } from '../../module';
import { makeNodeTransport } from '../../transports';
import type { WorkerStartData } from './common';

Expand Down Expand Up @@ -158,8 +160,9 @@ if (options.captureStackTrace) {
// copy the frames
const callFrames = [...event.params.callFrames];

const getModuleName = options.appRootPath ? createGetModuleFromFilename(options.appRootPath) : () => undefined;
const stackFrames = callFrames.map(frame =>
callFrameToStackFrame(frame, scripts.get(frame.location.scriptId), () => undefined),
callFrameToStackFrame(frame, scripts.get(frame.location.scriptId), getModuleName),
);

// Evaluate a script in the currently paused context
Expand Down
83 changes: 36 additions & 47 deletions packages/node/src/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,62 +8,51 @@ function normalizeWindowsPath(path: string): string {
.replace(/\\/g, '/'); // replace all `\` instances with `/`
}

// We cache this so we don't have to recompute it
let basePath: string | undefined;

function getBasePath(): string {
if (!basePath) {
const baseDir =
require && require.main && require.main.filename ? dirname(require.main.filename) : global.process.cwd();
basePath = `${baseDir}/`;
}

return basePath;
}

/** Gets the module from a filename */
export function getModuleFromFilename(
filename: string | undefined,
basePath: string = getBasePath(),
/** Creates a function that gets the module name from a filename */
export function createGetModuleFromFilename(
basePath: string = dirname(process.argv[1]),
isWindows: boolean = sep === '\\',
): string | undefined {
if (!filename) {
return;
}

): (filename: string | undefined) => string | undefined {
const normalizedBase = isWindows ? normalizeWindowsPath(basePath) : basePath;
const normalizedFilename = isWindows ? normalizeWindowsPath(filename) : filename;

// eslint-disable-next-line prefer-const
let { dir, base: file, ext } = posix.parse(normalizedFilename);
return (filename: string | undefined) => {
if (!filename) {
return;
}

if (ext === '.js' || ext === '.mjs' || ext === '.cjs') {
file = file.slice(0, ext.length * -1);
}
const normalizedFilename = isWindows ? normalizeWindowsPath(filename) : filename;

if (!dir) {
// No dirname whatsoever
dir = '.';
}
// eslint-disable-next-line prefer-const
let { dir, base: file, ext } = posix.parse(normalizedFilename);

let n = dir.lastIndexOf('/node_modules');
if (n > -1) {
return `${dir.slice(n + 14).replace(/\//g, '.')}:${file}`;
}
if (ext === '.js' || ext === '.mjs' || ext === '.cjs') {
file = file.slice(0, ext.length * -1);
}

// 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
n = `${dir}/`.lastIndexOf(normalizedBase, 0);
if (n === 0) {
let moduleName = dir.slice(normalizedBase.length).replace(/\//g, '.');
if (!dir) {
// No dirname whatsoever
dir = '.';
}

if (moduleName) {
moduleName += ':';
let n = dir.lastIndexOf('/node_modules');
if (n > -1) {
return `${dir.slice(n + 14).replace(/\//g, '.')}:${file}`;
}
moduleName += file;

return moduleName;
}
// 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
n = `${dir}/`.lastIndexOf(normalizedBase, 0);
if (n === 0) {
let moduleName = dir.slice(normalizedBase.length).replace(/\//g, '.');

if (moduleName) {
moduleName += ':';
}
moduleName += file;

return moduleName;
}

return file;
return file;
};
}
4 changes: 2 additions & 2 deletions packages/node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {
Spotlight,
Undici,
} from './integrations';
import { getModuleFromFilename } from './module';
import { createGetModuleFromFilename } from './module';
import { makeNodeTransport } from './transports';
import type { NodeClientOptions, NodeOptions } from './types';

Expand Down Expand Up @@ -240,7 +240,7 @@ export function getSentryRelease(fallback?: string): string | undefined {
}

/** Node.js stack parser */
export const defaultStackParser: StackParser = createStackParser(nodeStackLineParser(getModuleFromFilename));
export const defaultStackParser: StackParser = createStackParser(nodeStackLineParser(createGetModuleFromFilename()));

/**
* Enable automatic Session Tracking for the node process.
Expand Down
39 changes: 16 additions & 23 deletions packages/node/test/module.test.ts
Original file line number Diff line number Diff line change
@@ -1,40 +1,33 @@
import { getModuleFromFilename } from '../src/module';
import { createGetModuleFromFilename } from '../src/module';

describe('getModuleFromFilename', () => {
test('Windows', () => {
expect(
getModuleFromFilename('C:\\Users\\Tim\\node_modules\\some-dep\\module.js', 'C:\\Users\\Tim\\', true),
).toEqual('some-dep:module');
const getModuleFromFilenameWindows = createGetModuleFromFilename('C:\\Users\\Tim\\', true);
const getModuleFromFilenamePosix = createGetModuleFromFilename('/Users/Tim/');

expect(getModuleFromFilename('C:\\Users\\Tim\\some\\more\\feature.js', 'C:\\Users\\Tim\\', true)).toEqual(
'some.more:feature',
describe('createGetModuleFromFilename', () => {
test('Windows', () => {
expect(getModuleFromFilenameWindows('C:\\Users\\Tim\\node_modules\\some-dep\\module.js')).toEqual(
'some-dep:module',
);
expect(getModuleFromFilenameWindows('C:\\Users\\Tim\\some\\more\\feature.js')).toEqual('some.more:feature');
});

test('POSIX', () => {
expect(getModuleFromFilename('/Users/Tim/node_modules/some-dep/module.js', '/Users/Tim/')).toEqual(
'some-dep:module',
);

expect(getModuleFromFilename('/Users/Tim/some/more/feature.js', '/Users/Tim/')).toEqual('some.more:feature');
expect(getModuleFromFilename('/Users/Tim/main.js', '/Users/Tim/')).toEqual('main');
expect(getModuleFromFilenamePosix('/Users/Tim/node_modules/some-dep/module.js')).toEqual('some-dep:module');
expect(getModuleFromFilenamePosix('/Users/Tim/some/more/feature.js')).toEqual('some.more:feature');
expect(getModuleFromFilenamePosix('/Users/Tim/main.js')).toEqual('main');
});

test('.mjs', () => {
expect(getModuleFromFilename('/Users/Tim/node_modules/some-dep/module.mjs', '/Users/Tim/')).toEqual(
'some-dep:module',
);
expect(getModuleFromFilenamePosix('/Users/Tim/node_modules/some-dep/module.mjs')).toEqual('some-dep:module');
});

test('.cjs', () => {
expect(getModuleFromFilename('/Users/Tim/node_modules/some-dep/module.cjs', '/Users/Tim/')).toEqual(
'some-dep:module',
);
expect(getModuleFromFilenamePosix('/Users/Tim/node_modules/some-dep/module.cjs')).toEqual('some-dep:module');
});

test('node internal', () => {
expect(getModuleFromFilename('node.js', '/Users/Tim/')).toEqual('node');
expect(getModuleFromFilename('node:internal/process/task_queues', '/Users/Tim/')).toEqual('task_queues');
expect(getModuleFromFilename('node:internal/timers', '/Users/Tim/')).toEqual('timers');
expect(getModuleFromFilenamePosix('node.js')).toEqual('node');
expect(getModuleFromFilenamePosix('node:internal/process/task_queues')).toEqual('task_queues');
expect(getModuleFromFilenamePosix('node:internal/timers')).toEqual('timers');
});
});

0 comments on commit eff57fa

Please sign in to comment.