From 52ce3447fa8f0c7a860c44420354cad413606c0c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 9 Jan 2025 09:55:47 +0000 Subject: [PATCH] fix(profiling/v8): Don't put `require`, `__filename` and `__dirname` on global object --- .github/workflows/build.yml | 3 +- .../{build.mjs => build-cjs.mjs} | 3 +- .../{build.shimmed.mjs => build-esm.mjs} | 13 +-- .../test-applications/node-profiling/index.ts | 4 +- .../node-profiling/package.json | 8 +- packages/profiling-node/package.json | 1 + packages/profiling-node/rollup.npm.config.mjs | 51 +++-------- packages/profiling-node/src/cpu_profiler.ts | 86 +++++++++++-------- packages/profiling-node/tsconfig.json | 1 - 9 files changed, 72 insertions(+), 98 deletions(-) rename dev-packages/e2e-tests/test-applications/node-profiling/{build.mjs => build-cjs.mjs} (90%) rename dev-packages/e2e-tests/test-applications/node-profiling/{build.shimmed.mjs => build-esm.mjs} (68%) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 31f5cea28bda..d633b9bc784d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1209,7 +1209,8 @@ jobs: - name: Run E2E test working-directory: dev-packages/e2e-tests/test-applications/${{ matrix.test-application }} timeout-minutes: 10 - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- yarn test:assert + run: | + xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- yarn test:assert job_required_jobs_passed: name: All required jobs passed or were skipped diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/build.mjs b/dev-packages/e2e-tests/test-applications/node-profiling/build-cjs.mjs similarity index 90% rename from dev-packages/e2e-tests/test-applications/node-profiling/build.mjs rename to dev-packages/e2e-tests/test-applications/node-profiling/build-cjs.mjs index 55ec0b5fae52..4a9aa83d0eec 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/build.mjs +++ b/dev-packages/e2e-tests/test-applications/node-profiling/build-cjs.mjs @@ -11,9 +11,10 @@ console.log('Running build using esbuild version', esbuild.version); esbuild.buildSync({ platform: 'node', entryPoints: ['./index.ts'], - outdir: './dist', + outfile: './dist/cjs/index.js', target: 'esnext', format: 'cjs', bundle: true, loader: { '.node': 'copy' }, + external: ['@sentry/node', '@sentry/profiling-node'], }); diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs b/dev-packages/e2e-tests/test-applications/node-profiling/build-esm.mjs similarity index 68% rename from dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs rename to dev-packages/e2e-tests/test-applications/node-profiling/build-esm.mjs index c45e30539bc0..294e53d50635 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs +++ b/dev-packages/e2e-tests/test-applications/node-profiling/build-esm.mjs @@ -11,19 +11,10 @@ console.log('Running build using esbuild version', esbuild.version); esbuild.buildSync({ platform: 'node', entryPoints: ['./index.ts'], - outfile: './dist/index.shimmed.mjs', + outfile: './dist/esm/index.mjs', target: 'esnext', format: 'esm', bundle: true, loader: { '.node': 'copy' }, - banner: { - js: ` - import { dirname } from 'node:path'; - import { fileURLToPath } from 'node:url'; - import { createRequire } from 'node:module'; - const require = createRequire(import.meta.url); - const __filename = fileURLToPath(import.meta.url); - const __dirname = dirname(__filename); - `, - }, + external: ['@sentry/node', '@sentry/profiling-node'], }); diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/index.ts b/dev-packages/e2e-tests/test-applications/node-profiling/index.ts index d49add80955c..e956a1d9de33 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/index.ts +++ b/dev-packages/e2e-tests/test-applications/node-profiling/index.ts @@ -1,5 +1,5 @@ -const Sentry = require('@sentry/node'); -const { nodeProfilingIntegration } = require('@sentry/profiling-node'); +import * as Sentry from '@sentry/node'; +import { nodeProfilingIntegration } from '@sentry/profiling-node'; const wait = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/package.json b/dev-packages/e2e-tests/test-applications/node-profiling/package.json index 8aede827a1f3..d78ca10fa25d 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/package.json +++ b/dev-packages/e2e-tests/test-applications/node-profiling/package.json @@ -4,8 +4,8 @@ "private": true, "scripts": { "typecheck": "tsc --noEmit", - "build": "node build.mjs && node build.shimmed.mjs", - "test": "node dist/index.js && node --experimental-require-module dist/index.js && node dist/index.shimmed.mjs", + "build": "node build-cjs.mjs && node build-esm.mjs", + "test": "node dist/cjs/index.js && node --experimental-require-module dist/cjs/index.js && node dist/esm/index.mjs", "clean": "npx rimraf node_modules dist", "test:electron": "$(pnpm bin)/electron-rebuild && playwright test", "test:build": "pnpm run typecheck && pnpm run build", @@ -17,9 +17,9 @@ "@sentry/electron": "latest || *", "@sentry/node": "latest || *", "@sentry/profiling-node": "latest || *", - "electron": "^33.2.0" + "electron": "^33.2.0", + "esbuild": "0.20.0" }, - "devDependencies": {}, "volta": { "extends": "../../package.json" }, diff --git a/packages/profiling-node/package.json b/packages/profiling-node/package.json index f2a7090daf9e..369254b2e3ee 100644 --- a/packages/profiling-node/package.json +++ b/packages/profiling-node/package.json @@ -7,6 +7,7 @@ "author": "Sentry", "license": "MIT", "main": "lib/cjs/index.js", + "module": "lib/esm/index.js", "types": "lib/types/index.d.ts", "exports": { "./package.json": "./package.json", diff --git a/packages/profiling-node/rollup.npm.config.mjs b/packages/profiling-node/rollup.npm.config.mjs index 12492b7c83e8..a9c148306709 100644 --- a/packages/profiling-node/rollup.npm.config.mjs +++ b/packages/profiling-node/rollup.npm.config.mjs @@ -1,49 +1,20 @@ import commonjs from '@rollup/plugin-commonjs'; +import replace from '@rollup/plugin-replace'; import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils'; -export const ESMShim = ` -import cjsUrl from 'node:url'; -import cjsPath from 'node:path'; -import cjsModule from 'node:module'; - -if(typeof __filename === 'undefined'){ - globalThis.__filename = cjsUrl.fileURLToPath(import.meta.url); -} - -if(typeof __dirname === 'undefined'){ - globalThis.__dirname = cjsPath.dirname(__filename); -} - -if(typeof require === 'undefined'){ - globalThis.require = cjsModule.createRequire(import.meta.url); -} -`; - -function makeESMShimPlugin(shim) { - return { - transform(code) { - const SHIM_REGEXP = /\/\/ #START_SENTRY_ESM_SHIM[\s\S]*?\/\/ #END_SENTRY_ESM_SHIM/; - return code.replace(SHIM_REGEXP, shim); - }, - }; -} - -const variants = makeNPMConfigVariants( +export default makeNPMConfigVariants( makeBaseNPMConfig({ packageSpecificConfig: { output: { dir: 'lib', preserveModules: false }, - plugins: [commonjs()], + plugins: [ + commonjs(), + replace({ + preventAssignment: false, + values: { + __IMPORT_META_URL_REPLACEMENT__: 'import.meta.url', + }, + }), + ], }, }), ); - -for (const variant of variants) { - if (variant.output.format === 'esm') { - variant.plugins.push(makeESMShimPlugin(ESMShim)); - } else { - // Remove the ESM shim comment - variant.plugins.push(makeESMShimPlugin('')); - } -} - -export default variants; diff --git a/packages/profiling-node/src/cpu_profiler.ts b/packages/profiling-node/src/cpu_profiler.ts index ed4ad83e7b31..a9a6d65ce191 100644 --- a/packages/profiling-node/src/cpu_profiler.ts +++ b/packages/profiling-node/src/cpu_profiler.ts @@ -1,6 +1,9 @@ +import { createRequire } from 'node:module'; import { arch as _arch, platform as _platform } from 'node:os'; import { join, resolve } from 'node:path'; +import { dirname } from 'node:path'; import { env, versions } from 'node:process'; +import { fileURLToPath, pathToFileURL } from 'node:url'; import { threadId } from 'node:worker_threads'; import { familySync } from 'detect-libc'; import { getAbi } from 'node-abi'; @@ -15,11 +18,7 @@ import type { } from './types'; import type { ProfileFormat } from './types'; -// #START_SENTRY_ESM_SHIM -// When building for ESM, we shim require to use createRequire and __dirname. -// We need to do this because .node extensions in esm are not supported. -// The comment below this line exists as a placeholder for where to insert the shim. -// #END_SENTRY_ESM_SHIM +declare const __IMPORT_META_URL_REPLACEMENT__: string; const stdlib = familySync(); const platform = process.env['BUILD_PLATFORM'] || _platform(); @@ -27,23 +26,32 @@ const arch = process.env['BUILD_ARCH'] || _arch(); const abi = getAbi(versions.node, 'node'); const identifier = [platform, arch, stdlib, abi].filter(c => c !== undefined && c !== null).join('-'); -const built_from_source_path = resolve(__dirname, '..', `./sentry_cpu_profiler-${identifier}`); - /** * Imports cpp bindings based on the current platform and architecture. */ // eslint-disable-next-line complexity export function importCppBindingsModule(): PrivateV8CpuProfilerBindings { + // We need to work around using import.meta.url directly with __IMPORT_META_URL_REPLACEMENT__ because jest complains about it. + const importMetaUrl = + typeof __IMPORT_META_URL_REPLACEMENT__ !== 'undefined' + ? // This case is always hit when the SDK is built + __IMPORT_META_URL_REPLACEMENT__ + : // This case is hit when the tests are run + pathToFileURL(__filename).href; + + const createdRequire = createRequire(importMetaUrl); + const esmCompatibleDirname = dirname(fileURLToPath(importMetaUrl)); + // If a binary path is specified, use that. if (env['SENTRY_PROFILER_BINARY_PATH']) { const envPath = env['SENTRY_PROFILER_BINARY_PATH']; - return require(envPath); + return createdRequire(envPath); } // If a user specifies a different binary dir, they are in control of the binaries being moved there if (env['SENTRY_PROFILER_BINARY_DIR']) { const binaryPath = join(resolve(env['SENTRY_PROFILER_BINARY_DIR']), `sentry_cpu_profiler-${identifier}`); - return require(`${binaryPath}.node`); + return createdRequire(`${binaryPath}.node`); } // We need the fallthrough so that in the end, we can fallback to the dynamic require. @@ -51,31 +59,31 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings { if (platform === 'darwin') { if (arch === 'x64') { if (abi === '93') { - return require('../sentry_cpu_profiler-darwin-x64-93.node'); + return createdRequire('../sentry_cpu_profiler-darwin-x64-93.node'); } if (abi === '108') { - return require('../sentry_cpu_profiler-darwin-x64-108.node'); + return createdRequire('../sentry_cpu_profiler-darwin-x64-108.node'); } if (abi === '115') { - return require('../sentry_cpu_profiler-darwin-x64-115.node'); + return createdRequire('../sentry_cpu_profiler-darwin-x64-115.node'); } if (abi === '127') { - return require('../sentry_cpu_profiler-darwin-x64-127.node'); + return createdRequire('../sentry_cpu_profiler-darwin-x64-127.node'); } } if (arch === 'arm64') { if (abi === '93') { - return require('../sentry_cpu_profiler-darwin-arm64-93.node'); + return createdRequire('../sentry_cpu_profiler-darwin-arm64-93.node'); } if (abi === '108') { - return require('../sentry_cpu_profiler-darwin-arm64-108.node'); + return createdRequire('../sentry_cpu_profiler-darwin-arm64-108.node'); } if (abi === '115') { - return require('../sentry_cpu_profiler-darwin-arm64-115.node'); + return createdRequire('../sentry_cpu_profiler-darwin-arm64-115.node'); } if (abi === '127') { - return require('../sentry_cpu_profiler-darwin-arm64-127.node'); + return createdRequire('../sentry_cpu_profiler-darwin-arm64-127.node'); } } } @@ -83,16 +91,16 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings { if (platform === 'win32') { if (arch === 'x64') { if (abi === '93') { - return require('../sentry_cpu_profiler-win32-x64-93.node'); + return createdRequire('../sentry_cpu_profiler-win32-x64-93.node'); } if (abi === '108') { - return require('../sentry_cpu_profiler-win32-x64-108.node'); + return createdRequire('../sentry_cpu_profiler-win32-x64-108.node'); } if (abi === '115') { - return require('../sentry_cpu_profiler-win32-x64-115.node'); + return createdRequire('../sentry_cpu_profiler-win32-x64-115.node'); } if (abi === '127') { - return require('../sentry_cpu_profiler-win32-x64-127.node'); + return createdRequire('../sentry_cpu_profiler-win32-x64-127.node'); } } } @@ -101,66 +109,68 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings { if (arch === 'x64') { if (stdlib === 'musl') { if (abi === '93') { - return require('../sentry_cpu_profiler-linux-x64-musl-93.node'); + return createdRequire('../sentry_cpu_profiler-linux-x64-musl-93.node'); } if (abi === '108') { - return require('../sentry_cpu_profiler-linux-x64-musl-108.node'); + return createdRequire('../sentry_cpu_profiler-linux-x64-musl-108.node'); } if (abi === '115') { - return require('../sentry_cpu_profiler-linux-x64-musl-115.node'); + return createdRequire('../sentry_cpu_profiler-linux-x64-musl-115.node'); } if (abi === '127') { - return require('../sentry_cpu_profiler-linux-x64-musl-127.node'); + return createdRequire('../sentry_cpu_profiler-linux-x64-musl-127.node'); } } if (stdlib === 'glibc') { if (abi === '93') { - return require('../sentry_cpu_profiler-linux-x64-glibc-93.node'); + return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-93.node'); } if (abi === '108') { - return require('../sentry_cpu_profiler-linux-x64-glibc-108.node'); + return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-108.node'); } if (abi === '115') { - return require('../sentry_cpu_profiler-linux-x64-glibc-115.node'); + return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-115.node'); } if (abi === '127') { - return require('../sentry_cpu_profiler-linux-x64-glibc-127.node'); + return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-127.node'); } } } if (arch === 'arm64') { if (stdlib === 'musl') { if (abi === '93') { - return require('../sentry_cpu_profiler-linux-arm64-musl-93.node'); + return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-93.node'); } if (abi === '108') { - return require('../sentry_cpu_profiler-linux-arm64-musl-108.node'); + return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-108.node'); } if (abi === '115') { - return require('../sentry_cpu_profiler-linux-arm64-musl-115.node'); + return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-115.node'); } if (abi === '127') { - return require('../sentry_cpu_profiler-linux-arm64-musl-127.node'); + return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-127.node'); } } if (stdlib === 'glibc') { if (abi === '93') { - return require('../sentry_cpu_profiler-linux-arm64-glibc-93.node'); + return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-93.node'); } if (abi === '108') { - return require('../sentry_cpu_profiler-linux-arm64-glibc-108.node'); + return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-108.node'); } if (abi === '115') { - return require('../sentry_cpu_profiler-linux-arm64-glibc-115.node'); + return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-115.node'); } if (abi === '127') { - return require('../sentry_cpu_profiler-linux-arm64-glibc-127.node'); + return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-127.node'); } } } } - return require(`${built_from_source_path}.node`); + + const built_from_source_path = resolve(esmCompatibleDirname, '..', `sentry_cpu_profiler-${identifier}`); + return createdRequire(`${built_from_source_path}.node`); } const PrivateCpuProfilerBindings: PrivateV8CpuProfilerBindings = importCppBindingsModule(); diff --git a/packages/profiling-node/tsconfig.json b/packages/profiling-node/tsconfig.json index c53d22cf5270..68bd9a52df2a 100644 --- a/packages/profiling-node/tsconfig.json +++ b/packages/profiling-node/tsconfig.json @@ -8,4 +8,3 @@ }, "include": ["src/**/*"] } -