Skip to content

Commit 0770e69

Browse files
chargomeclaude
andcommitted
fix(core): Guard loadModule default parameter against ESM scope
Default parameters are evaluated before the function body, so the bare `module` default threw `ReferenceError: module is not defined` from the ESM build before reaching the try/catch that is meant to make `loadModule` degrade gracefully. Guard it with a `typeof module` check so ESM callers get `undefined` instead. Fixes #24117 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 37fd6a5 commit 0770e69

4 files changed

Lines changed: 46 additions & 2 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { loadModule } from '@sentry/core/server';
2+
3+
// The default `existingModule` argument must not reference a CJS-only binding: default
4+
// parameters are evaluated before the function body, so a bare `module` would throw
5+
// outside the try/catch that is supposed to make this helper degrade gracefully.
6+
loadModule('node:path');
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { afterAll, describe, test } from 'vitest';
2+
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
3+
4+
afterAll(() => {
5+
cleanupChildProcesses();
6+
});
7+
8+
describe('loadModule', () => {
9+
test('does not throw when called without `existingModule` from ESM', async () => {
10+
await createRunner(__dirname, 'app.mjs').ensureNoErrorOutput().start().completed();
11+
});
12+
});

‎packages/core/src/utils/node.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,14 @@ function dynamicRequire(mod: any, request: string): any {
4444
* @param existingModule module to use for requiring
4545
* @returns possibly required module
4646
*/
47-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
48-
export function loadModule<T>(moduleName: string, existingModule: any = module): T | undefined {
47+
export function loadModule<T>(
48+
moduleName: string,
49+
// Default parameters are evaluated before the body runs, so a bare `module` would throw a
50+
// ReferenceError in ESM before reaching the try/catch below that makes this helper degrade
51+
// gracefully. Guard it so the ESM build resolves to `undefined` instead of crashing.
52+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
53+
existingModule: any = typeof module !== 'undefined' ? module : undefined,
54+
): T | undefined {
4955
let mod: T | undefined;
5056

5157
try {
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { loadModule } from '../../../src/utils/node';
3+
4+
// vitest's `module` shim has no `require`, so tests hand in an explicit CJS-like module object.
5+
const cjsModule = { require };
6+
7+
describe('loadModule', () => {
8+
it('loads a module via the given `existingModule`', () => {
9+
const path = loadModule<{ join: unknown }>('path', cjsModule);
10+
expect(path?.join).toBeTypeOf('function');
11+
});
12+
13+
it('returns undefined for a module that cannot be resolved', () => {
14+
expect(loadModule('@sentry/this-module-does-not-exist', cjsModule)).toBeUndefined();
15+
});
16+
17+
it('returns undefined instead of throwing when `existingModule` cannot require', () => {
18+
expect(loadModule('path', undefined)).toBeUndefined();
19+
});
20+
});

0 commit comments

Comments
 (0)