Skip to content

Commit 1c65788

Browse files
authored
feat(node): Tidy existing ESM loader hook (#17566)
I was looking to add another loader hook but I found some issues with the existing code: - To ensure the hook only gets registered once, `GLOBAL_OBJ._sentryEsmLoaderHookRegistered` is used but it was never set to true anywhere - Hook registration was duplicated in `@sentry/node-core` and `@sentry/node` for preloading - I moved the Node version check/warning to `supportsEsmLoaderHooks()` so it can be reused in multiple hooks
1 parent 4c7d65e commit 1c65788

File tree

9 files changed

+82
-77
lines changed

9 files changed

+82
-77
lines changed

packages/node-core/src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ export { getSentryRelease, defaultStackParser } from './sdk/api';
3535
export { createGetModuleFromFilename } from './utils/module';
3636
export { addOriginToSpan } from './utils/addOriginToSpan';
3737
export { getRequestUrl } from './utils/getRequestUrl';
38-
export { isCjs } from './utils/commonjs';
38+
export { initializeEsmLoader } from './sdk/esmLoader';
39+
export { isCjs } from './utils/detection';
3940
export { ensureIsWrapped } from './utils/ensureIsWrapped';
4041
export { createMissingInstrumentationContext } from './utils/createMissingInstrumentationContext';
4142
export { envToBool } from './utils/envToBool';

packages/node-core/src/integrations/modules.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { existsSync, readFileSync } from 'node:fs';
22
import { dirname, join } from 'node:path';
33
import type { IntegrationFn } from '@sentry/core';
4-
import { isCjs } from '../utils/commonjs';
4+
import { isCjs } from '../utils/detection';
55

66
type ModuleInfo = Record<string, string>;
77

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,31 @@
1-
import { consoleSandbox, debug, GLOBAL_OBJ } from '@sentry/core';
1+
import { debug, GLOBAL_OBJ } from '@sentry/core';
22
import { createAddHookMessageChannel } from 'import-in-the-middle';
3-
import moduleModule from 'module';
3+
import * as moduleModule from 'module';
4+
import { supportsEsmLoaderHooks } from '../utils/detection';
45

5-
/** Initialize the ESM loader. */
6-
export function maybeInitializeEsmLoader(): void {
7-
const [nodeMajor = 0, nodeMinor = 0] = process.versions.node.split('.').map(Number);
6+
/**
7+
* Initialize the ESM loader - This method is private and not part of the public
8+
* API.
9+
*
10+
* @ignore
11+
*/
12+
export function initializeEsmLoader(): void {
13+
if (!supportsEsmLoaderHooks()) {
14+
return;
15+
}
16+
17+
if (!GLOBAL_OBJ._sentryEsmLoaderHookRegistered) {
18+
GLOBAL_OBJ._sentryEsmLoaderHookRegistered = true;
819

9-
// Register hook was added in v20.6.0 and v18.19.0
10-
if (nodeMajor >= 21 || (nodeMajor === 20 && nodeMinor >= 6) || (nodeMajor === 18 && nodeMinor >= 19)) {
11-
if (!GLOBAL_OBJ._sentryEsmLoaderHookRegistered) {
12-
try {
13-
const { addHookMessagePort } = createAddHookMessageChannel();
14-
// @ts-expect-error register is available in these versions
15-
moduleModule.register('import-in-the-middle/hook.mjs', import.meta.url, {
16-
data: { addHookMessagePort, include: [] },
17-
transferList: [addHookMessagePort],
18-
});
19-
} catch (error) {
20-
debug.warn('Failed to register ESM hook', error);
21-
}
20+
try {
21+
const { addHookMessagePort } = createAddHookMessageChannel();
22+
// @ts-expect-error register is available in these versions
23+
moduleModule.register('import-in-the-middle/hook.mjs', import.meta.url, {
24+
data: { addHookMessagePort, include: [] },
25+
transferList: [addHookMessagePort],
26+
});
27+
} catch (error) {
28+
debug.warn("Failed to register 'import-in-the-middle' hook", error);
2229
}
23-
} else {
24-
consoleSandbox(() => {
25-
// eslint-disable-next-line no-console
26-
console.warn(
27-
`[Sentry] You are using Node.js v${process.versions.node} in ESM mode ("import syntax"). The Sentry Node.js SDK is not compatible with ESM in Node.js versions before 18.19.0 or before 20.6.0. Please either build your application with CommonJS ("require() syntax"), or upgrade your Node.js version.`,
28-
);
29-
});
3030
}
3131
}

packages/node-core/src/sdk/index.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ import { INTEGRATION_NAME as SPOTLIGHT_INTEGRATION_NAME, spotlightIntegration }
3535
import { systemErrorIntegration } from '../integrations/systemError';
3636
import { makeNodeTransport } from '../transports';
3737
import type { NodeClientOptions, NodeOptions } from '../types';
38-
import { isCjs } from '../utils/commonjs';
38+
import { isCjs } from '../utils/detection';
3939
import { envToBool } from '../utils/envToBool';
4040
import { defaultStackParser, getSentryRelease } from './api';
4141
import { NodeClient } from './client';
42-
import { maybeInitializeEsmLoader } from './esmLoader';
42+
import { initializeEsmLoader } from './esmLoader';
4343

4444
/**
4545
* Get default integrations for the Node-Core SDK.
@@ -106,8 +106,8 @@ function _init(
106106
}
107107
}
108108

109-
if (!isCjs() && options.registerEsmLoaderHooks !== false) {
110-
maybeInitializeEsmLoader();
109+
if (options.registerEsmLoaderHooks !== false) {
110+
initializeEsmLoader();
111111
}
112112

113113
setOpenTelemetryContextAsyncContextStrategy();
@@ -131,7 +131,7 @@ function _init(
131131

132132
client.init();
133133

134-
debug.log(`Running in ${isCjs() ? 'CommonJS' : 'ESM'} mode.`);
134+
debug.log(`SDK initialized from ${isCjs() ? 'CommonJS' : 'ESM'}`);
135135

136136
client.startClientReportTracking();
137137

packages/node-core/src/utils/commonjs.ts

Lines changed: 0 additions & 8 deletions
This file was deleted.

packages/node-core/src/utils/createMissingInstrumentationContext.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { MissingInstrumentationContext } from '@sentry/core';
2-
import { isCjs } from './commonjs';
2+
import { isCjs } from './detection';
33

44
export const createMissingInstrumentationContext = (pkg: string): MissingInstrumentationContext => ({
55
package: pkg,
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { consoleSandbox } from '@sentry/core';
2+
import { NODE_MAJOR, NODE_MINOR } from '../nodeVersion';
3+
4+
/** Detect CommonJS. */
5+
export function isCjs(): boolean {
6+
try {
7+
return typeof module !== 'undefined' && typeof module.exports !== 'undefined';
8+
} catch {
9+
return false;
10+
}
11+
}
12+
13+
let hasWarnedAboutNodeVersion: boolean | undefined;
14+
15+
/**
16+
* Check if the current Node.js version supports module.register
17+
*/
18+
export function supportsEsmLoaderHooks(): boolean {
19+
if (isCjs()) {
20+
return false;
21+
}
22+
23+
if (NODE_MAJOR >= 21 || (NODE_MAJOR === 20 && NODE_MINOR >= 6) || (NODE_MAJOR === 18 && NODE_MINOR >= 19)) {
24+
return true;
25+
}
26+
27+
if (!hasWarnedAboutNodeVersion) {
28+
hasWarnedAboutNodeVersion = true;
29+
30+
consoleSandbox(() => {
31+
// eslint-disable-next-line no-console
32+
console.warn(
33+
`[Sentry] You are using Node.js v${process.versions.node} in ESM mode ("import syntax"). The Sentry Node.js SDK is not compatible with ESM in Node.js versions before 18.19.0 or before 20.6.0. Please either build your application with CommonJS ("require() syntax"), or upgrade your Node.js version.`,
34+
);
35+
});
36+
}
37+
38+
return false;
39+
}

packages/node-core/src/utils/ensureIsWrapped.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { isWrapped } from '@opentelemetry/instrumentation';
22
import { consoleSandbox, getClient, getGlobalScope, hasSpansEnabled, isEnabled } from '@sentry/core';
33
import type { NodeClient } from '../sdk/client';
4-
import { isCjs } from './commonjs';
54
import { createMissingInstrumentationContext } from './createMissingInstrumentationContext';
5+
import { isCjs } from './detection';
66

77
/**
88
* Checks and warns if a framework isn't wrapped by opentelemetry.

packages/node/src/sdk/initOtel.ts

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,14 @@ import {
77
ATTR_SERVICE_VERSION,
88
SEMRESATTRS_SERVICE_NAMESPACE,
99
} from '@opentelemetry/semantic-conventions';
10-
import { consoleSandbox, debug as coreDebug, GLOBAL_OBJ, SDK_VERSION } from '@sentry/core';
11-
import { type NodeClient, isCjs, SentryContextManager, setupOpenTelemetryLogger } from '@sentry/node-core';
10+
import { debug as coreDebug, SDK_VERSION } from '@sentry/core';
11+
import {
12+
type NodeClient,
13+
initializeEsmLoader,
14+
SentryContextManager,
15+
setupOpenTelemetryLogger,
16+
} from '@sentry/node-core';
1217
import { SentryPropagator, SentrySampler, SentrySpanProcessor } from '@sentry/opentelemetry';
13-
import { createAddHookMessageChannel } from 'import-in-the-middle';
14-
import moduleModule from 'module';
1518
import { DEBUG_BUILD } from '../debug-build';
1619
import { getOpenTelemetryInstrumentationToPreload } from '../integrations/tracing';
1720

@@ -35,34 +38,6 @@ export function initOpenTelemetry(client: NodeClient, options: AdditionalOpenTel
3538
client.traceProvider = provider;
3639
}
3740

38-
/** Initialize the ESM loader. */
39-
export function maybeInitializeEsmLoader(): void {
40-
const [nodeMajor = 0, nodeMinor = 0] = process.versions.node.split('.').map(Number);
41-
42-
// Register hook was added in v20.6.0 and v18.19.0
43-
if (nodeMajor >= 21 || (nodeMajor === 20 && nodeMinor >= 6) || (nodeMajor === 18 && nodeMinor >= 19)) {
44-
if (!GLOBAL_OBJ._sentryEsmLoaderHookRegistered) {
45-
try {
46-
const { addHookMessagePort } = createAddHookMessageChannel();
47-
// @ts-expect-error register is available in these versions
48-
moduleModule.register('import-in-the-middle/hook.mjs', import.meta.url, {
49-
data: { addHookMessagePort, include: [] },
50-
transferList: [addHookMessagePort],
51-
});
52-
} catch (error) {
53-
coreDebug.warn('Failed to register ESM hook', error);
54-
}
55-
}
56-
} else {
57-
consoleSandbox(() => {
58-
// eslint-disable-next-line no-console
59-
console.warn(
60-
`[Sentry] You are using Node.js v${process.versions.node} in ESM mode ("import syntax"). The Sentry Node.js SDK is not compatible with ESM in Node.js versions before 18.19.0 or before 20.6.0. Please either build your application with CommonJS ("require() syntax"), or upgrade your Node.js version.`,
61-
);
62-
});
63-
}
64-
}
65-
6641
interface NodePreloadOptions {
6742
debug?: boolean;
6843
integrations?: string[];
@@ -80,9 +55,7 @@ export function preloadOpenTelemetry(options: NodePreloadOptions = {}): void {
8055
coreDebug.enable();
8156
}
8257

83-
if (!isCjs()) {
84-
maybeInitializeEsmLoader();
85-
}
58+
initializeEsmLoader();
8659

8760
// These are all integrations that we need to pre-load to ensure they are set up before any other code runs
8861
getPreloadMethods(options.integrations).forEach(fn => {

0 commit comments

Comments
 (0)