Skip to content

Commit 785aab3

Browse files
lforstlforst
authored
fix(profiling/v8): Don't put require, __filename and __dirname on global object (#14952)
Backports #14470 to v8. Fixes #14525 Fixes #13662
1 parent e31e1c5 commit 785aab3

File tree

9 files changed

+72
-98
lines changed

9 files changed

+72
-98
lines changed

‎.github/workflows/build.yml

+2-1
Original file line numberDiff line numberDiff line change
@@ -1209,7 +1209,8 @@ jobs:
12091209
- name: Run E2E test
12101210
working-directory: dev-packages/e2e-tests/test-applications/${{ matrix.test-application }}
12111211
timeout-minutes: 10
1212-
run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- yarn test:assert
1212+
run: |
1213+
xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- yarn test:assert
12131214
12141215
job_required_jobs_passed:
12151216
name: All required jobs passed or were skipped

‎dev-packages/e2e-tests/test-applications/node-profiling/build.mjs ‎dev-packages/e2e-tests/test-applications/node-profiling/build-cjs.mjs

+2-1
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ console.log('Running build using esbuild version', esbuild.version);
1111
esbuild.buildSync({
1212
platform: 'node',
1313
entryPoints: ['./index.ts'],
14-
outdir: './dist',
14+
outfile: './dist/cjs/index.js',
1515
target: 'esnext',
1616
format: 'cjs',
1717
bundle: true,
1818
loader: { '.node': 'copy' },
19+
external: ['@sentry/node', '@sentry/profiling-node'],
1920
});

‎dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs ‎dev-packages/e2e-tests/test-applications/node-profiling/build-esm.mjs

+2-11
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,10 @@ console.log('Running build using esbuild version', esbuild.version);
1111
esbuild.buildSync({
1212
platform: 'node',
1313
entryPoints: ['./index.ts'],
14-
outfile: './dist/index.shimmed.mjs',
14+
outfile: './dist/esm/index.mjs',
1515
target: 'esnext',
1616
format: 'esm',
1717
bundle: true,
1818
loader: { '.node': 'copy' },
19-
banner: {
20-
js: `
21-
import { dirname } from 'node:path';
22-
import { fileURLToPath } from 'node:url';
23-
import { createRequire } from 'node:module';
24-
const require = createRequire(import.meta.url);
25-
const __filename = fileURLToPath(import.meta.url);
26-
const __dirname = dirname(__filename);
27-
`,
28-
},
19+
external: ['@sentry/node', '@sentry/profiling-node'],
2920
});

‎dev-packages/e2e-tests/test-applications/node-profiling/index.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
const Sentry = require('@sentry/node');
2-
const { nodeProfilingIntegration } = require('@sentry/profiling-node');
1+
import * as Sentry from '@sentry/node';
2+
import { nodeProfilingIntegration } from '@sentry/profiling-node';
33

44
const wait = (ms: number) => new Promise(resolve => setTimeout(resolve, ms));
55

‎dev-packages/e2e-tests/test-applications/node-profiling/package.json

+4-4
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
"private": true,
55
"scripts": {
66
"typecheck": "tsc --noEmit",
7-
"build": "node build.mjs && node build.shimmed.mjs",
8-
"test": "node dist/index.js && node --experimental-require-module dist/index.js && node dist/index.shimmed.mjs",
7+
"build": "node build-cjs.mjs && node build-esm.mjs",
8+
"test": "node dist/cjs/index.js && node --experimental-require-module dist/cjs/index.js && node dist/esm/index.mjs",
99
"clean": "npx rimraf node_modules dist",
1010
"test:electron": "$(pnpm bin)/electron-rebuild && playwright test",
1111
"test:build": "pnpm run typecheck && pnpm run build",
@@ -17,9 +17,9 @@
1717
"@sentry/electron": "latest || *",
1818
"@sentry/node": "latest || *",
1919
"@sentry/profiling-node": "latest || *",
20-
"electron": "^33.2.0"
20+
"electron": "^33.2.0",
21+
"esbuild": "0.20.0"
2122
},
22-
"devDependencies": {},
2323
"volta": {
2424
"extends": "../../package.json"
2525
},

‎packages/profiling-node/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"author": "Sentry",
88
"license": "MIT",
99
"main": "lib/cjs/index.js",
10+
"module": "lib/esm/index.js",
1011
"types": "lib/types/index.d.ts",
1112
"exports": {
1213
"./package.json": "./package.json",

‎packages/profiling-node/rollup.npm.config.mjs

+11-40
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,20 @@
11
import commonjs from '@rollup/plugin-commonjs';
2+
import replace from '@rollup/plugin-replace';
23
import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils';
34

4-
export const ESMShim = `
5-
import cjsUrl from 'node:url';
6-
import cjsPath from 'node:path';
7-
import cjsModule from 'node:module';
8-
9-
if(typeof __filename === 'undefined'){
10-
globalThis.__filename = cjsUrl.fileURLToPath(import.meta.url);
11-
}
12-
13-
if(typeof __dirname === 'undefined'){
14-
globalThis.__dirname = cjsPath.dirname(__filename);
15-
}
16-
17-
if(typeof require === 'undefined'){
18-
globalThis.require = cjsModule.createRequire(import.meta.url);
19-
}
20-
`;
21-
22-
function makeESMShimPlugin(shim) {
23-
return {
24-
transform(code) {
25-
const SHIM_REGEXP = /\/\/ #START_SENTRY_ESM_SHIM[\s\S]*?\/\/ #END_SENTRY_ESM_SHIM/;
26-
return code.replace(SHIM_REGEXP, shim);
27-
},
28-
};
29-
}
30-
31-
const variants = makeNPMConfigVariants(
5+
export default makeNPMConfigVariants(
326
makeBaseNPMConfig({
337
packageSpecificConfig: {
348
output: { dir: 'lib', preserveModules: false },
35-
plugins: [commonjs()],
9+
plugins: [
10+
commonjs(),
11+
replace({
12+
preventAssignment: false,
13+
values: {
14+
__IMPORT_META_URL_REPLACEMENT__: 'import.meta.url',
15+
},
16+
}),
17+
],
3618
},
3719
}),
3820
);
39-
40-
for (const variant of variants) {
41-
if (variant.output.format === 'esm') {
42-
variant.plugins.push(makeESMShimPlugin(ESMShim));
43-
} else {
44-
// Remove the ESM shim comment
45-
variant.plugins.push(makeESMShimPlugin(''));
46-
}
47-
}
48-
49-
export default variants;

‎packages/profiling-node/src/cpu_profiler.ts

+48-38
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import { createRequire } from 'node:module';
12
import { arch as _arch, platform as _platform } from 'node:os';
23
import { join, resolve } from 'node:path';
4+
import { dirname } from 'node:path';
35
import { env, versions } from 'node:process';
6+
import { fileURLToPath, pathToFileURL } from 'node:url';
47
import { threadId } from 'node:worker_threads';
58
import { familySync } from 'detect-libc';
69
import { getAbi } from 'node-abi';
@@ -15,84 +18,89 @@ import type {
1518
} from './types';
1619
import type { ProfileFormat } from './types';
1720

18-
// #START_SENTRY_ESM_SHIM
19-
// When building for ESM, we shim require to use createRequire and __dirname.
20-
// We need to do this because .node extensions in esm are not supported.
21-
// The comment below this line exists as a placeholder for where to insert the shim.
22-
// #END_SENTRY_ESM_SHIM
21+
declare const __IMPORT_META_URL_REPLACEMENT__: string;
2322

2423
const stdlib = familySync();
2524
const platform = process.env['BUILD_PLATFORM'] || _platform();
2625
const arch = process.env['BUILD_ARCH'] || _arch();
2726
const abi = getAbi(versions.node, 'node');
2827
const identifier = [platform, arch, stdlib, abi].filter(c => c !== undefined && c !== null).join('-');
2928

30-
const built_from_source_path = resolve(__dirname, '..', `./sentry_cpu_profiler-${identifier}`);
31-
3229
/**
3330
* Imports cpp bindings based on the current platform and architecture.
3431
*/
3532
// eslint-disable-next-line complexity
3633
export function importCppBindingsModule(): PrivateV8CpuProfilerBindings {
34+
// We need to work around using import.meta.url directly with __IMPORT_META_URL_REPLACEMENT__ because jest complains about it.
35+
const importMetaUrl =
36+
typeof __IMPORT_META_URL_REPLACEMENT__ !== 'undefined'
37+
? // This case is always hit when the SDK is built
38+
__IMPORT_META_URL_REPLACEMENT__
39+
: // This case is hit when the tests are run
40+
pathToFileURL(__filename).href;
41+
42+
const createdRequire = createRequire(importMetaUrl);
43+
const esmCompatibleDirname = dirname(fileURLToPath(importMetaUrl));
44+
3745
// If a binary path is specified, use that.
3846
if (env['SENTRY_PROFILER_BINARY_PATH']) {
3947
const envPath = env['SENTRY_PROFILER_BINARY_PATH'];
40-
return require(envPath);
48+
return createdRequire(envPath);
4149
}
4250

4351
// If a user specifies a different binary dir, they are in control of the binaries being moved there
4452
if (env['SENTRY_PROFILER_BINARY_DIR']) {
4553
const binaryPath = join(resolve(env['SENTRY_PROFILER_BINARY_DIR']), `sentry_cpu_profiler-${identifier}`);
46-
return require(`${binaryPath}.node`);
54+
return createdRequire(`${binaryPath}.node`);
4755
}
4856

4957
// We need the fallthrough so that in the end, we can fallback to the dynamic require.
5058
// This is for cases where precompiled binaries were not provided, but may have been compiled from source.
5159
if (platform === 'darwin') {
5260
if (arch === 'x64') {
5361
if (abi === '93') {
54-
return require('../sentry_cpu_profiler-darwin-x64-93.node');
62+
return createdRequire('../sentry_cpu_profiler-darwin-x64-93.node');
5563
}
5664
if (abi === '108') {
57-
return require('../sentry_cpu_profiler-darwin-x64-108.node');
65+
return createdRequire('../sentry_cpu_profiler-darwin-x64-108.node');
5866
}
5967
if (abi === '115') {
60-
return require('../sentry_cpu_profiler-darwin-x64-115.node');
68+
return createdRequire('../sentry_cpu_profiler-darwin-x64-115.node');
6169
}
6270
if (abi === '127') {
63-
return require('../sentry_cpu_profiler-darwin-x64-127.node');
71+
return createdRequire('../sentry_cpu_profiler-darwin-x64-127.node');
6472
}
6573
}
6674

6775
if (arch === 'arm64') {
6876
if (abi === '93') {
69-
return require('../sentry_cpu_profiler-darwin-arm64-93.node');
77+
return createdRequire('../sentry_cpu_profiler-darwin-arm64-93.node');
7078
}
7179
if (abi === '108') {
72-
return require('../sentry_cpu_profiler-darwin-arm64-108.node');
80+
return createdRequire('../sentry_cpu_profiler-darwin-arm64-108.node');
7381
}
7482
if (abi === '115') {
75-
return require('../sentry_cpu_profiler-darwin-arm64-115.node');
83+
return createdRequire('../sentry_cpu_profiler-darwin-arm64-115.node');
7684
}
7785
if (abi === '127') {
78-
return require('../sentry_cpu_profiler-darwin-arm64-127.node');
86+
return createdRequire('../sentry_cpu_profiler-darwin-arm64-127.node');
7987
}
8088
}
8189
}
8290

8391
if (platform === 'win32') {
8492
if (arch === 'x64') {
8593
if (abi === '93') {
86-
return require('../sentry_cpu_profiler-win32-x64-93.node');
94+
return createdRequire('../sentry_cpu_profiler-win32-x64-93.node');
8795
}
8896
if (abi === '108') {
89-
return require('../sentry_cpu_profiler-win32-x64-108.node');
97+
return createdRequire('../sentry_cpu_profiler-win32-x64-108.node');
9098
}
9199
if (abi === '115') {
92-
return require('../sentry_cpu_profiler-win32-x64-115.node');
100+
return createdRequire('../sentry_cpu_profiler-win32-x64-115.node');
93101
}
94102
if (abi === '127') {
95-
return require('../sentry_cpu_profiler-win32-x64-127.node');
103+
return createdRequire('../sentry_cpu_profiler-win32-x64-127.node');
96104
}
97105
}
98106
}
@@ -101,66 +109,68 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings {
101109
if (arch === 'x64') {
102110
if (stdlib === 'musl') {
103111
if (abi === '93') {
104-
return require('../sentry_cpu_profiler-linux-x64-musl-93.node');
112+
return createdRequire('../sentry_cpu_profiler-linux-x64-musl-93.node');
105113
}
106114
if (abi === '108') {
107-
return require('../sentry_cpu_profiler-linux-x64-musl-108.node');
115+
return createdRequire('../sentry_cpu_profiler-linux-x64-musl-108.node');
108116
}
109117
if (abi === '115') {
110-
return require('../sentry_cpu_profiler-linux-x64-musl-115.node');
118+
return createdRequire('../sentry_cpu_profiler-linux-x64-musl-115.node');
111119
}
112120
if (abi === '127') {
113-
return require('../sentry_cpu_profiler-linux-x64-musl-127.node');
121+
return createdRequire('../sentry_cpu_profiler-linux-x64-musl-127.node');
114122
}
115123
}
116124
if (stdlib === 'glibc') {
117125
if (abi === '93') {
118-
return require('../sentry_cpu_profiler-linux-x64-glibc-93.node');
126+
return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-93.node');
119127
}
120128
if (abi === '108') {
121-
return require('../sentry_cpu_profiler-linux-x64-glibc-108.node');
129+
return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-108.node');
122130
}
123131
if (abi === '115') {
124-
return require('../sentry_cpu_profiler-linux-x64-glibc-115.node');
132+
return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-115.node');
125133
}
126134
if (abi === '127') {
127-
return require('../sentry_cpu_profiler-linux-x64-glibc-127.node');
135+
return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-127.node');
128136
}
129137
}
130138
}
131139
if (arch === 'arm64') {
132140
if (stdlib === 'musl') {
133141
if (abi === '93') {
134-
return require('../sentry_cpu_profiler-linux-arm64-musl-93.node');
142+
return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-93.node');
135143
}
136144
if (abi === '108') {
137-
return require('../sentry_cpu_profiler-linux-arm64-musl-108.node');
145+
return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-108.node');
138146
}
139147
if (abi === '115') {
140-
return require('../sentry_cpu_profiler-linux-arm64-musl-115.node');
148+
return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-115.node');
141149
}
142150
if (abi === '127') {
143-
return require('../sentry_cpu_profiler-linux-arm64-musl-127.node');
151+
return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-127.node');
144152
}
145153
}
146154

147155
if (stdlib === 'glibc') {
148156
if (abi === '93') {
149-
return require('../sentry_cpu_profiler-linux-arm64-glibc-93.node');
157+
return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-93.node');
150158
}
151159
if (abi === '108') {
152-
return require('../sentry_cpu_profiler-linux-arm64-glibc-108.node');
160+
return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-108.node');
153161
}
154162
if (abi === '115') {
155-
return require('../sentry_cpu_profiler-linux-arm64-glibc-115.node');
163+
return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-115.node');
156164
}
157165
if (abi === '127') {
158-
return require('../sentry_cpu_profiler-linux-arm64-glibc-127.node');
166+
return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-127.node');
159167
}
160168
}
161169
}
162170
}
163-
return require(`${built_from_source_path}.node`);
171+
172+
const built_from_source_path = resolve(esmCompatibleDirname, '..', `sentry_cpu_profiler-${identifier}`);
173+
return createdRequire(`${built_from_source_path}.node`);
164174
}
165175

166176
const PrivateCpuProfilerBindings: PrivateV8CpuProfilerBindings = importCppBindingsModule();

‎packages/profiling-node/tsconfig.json

-1
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,3 @@
88
},
99
"include": ["src/**/*"]
1010
}
11-

0 commit comments

Comments
 (0)