Content-Length: 15896 | pFad | http://github.com/getsentry/sentry-javascript/pull/16566.patch

thub.com From a09699e6a3aecbbc380f179c948a70d042c6736a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 12 Jun 2025 13:50:15 +0200 Subject: [PATCH 1/3] feat(node): Ensure `modulesIntegration` works in more environments --- .../tests/server-components.test.ts | 8 ++ .../suites/modules/instrument.mjs | 8 ++ .../suites/modules/server.js | 22 +++++ .../suites/modules/server.mjs | 13 +++ .../suites/modules/test.ts | 48 ++++++++++ packages/nextjs/src/config/webpack.ts | 26 ++++++ packages/node/src/integrations/modules.ts | 91 ++++++++++++------- packages/node/src/sdk/index.ts | 6 +- 8 files changed, 186 insertions(+), 36 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/modules/instrument.mjs create mode 100644 dev-packages/node-integration-tests/suites/modules/server.js create mode 100644 dev-packages/node-integration-tests/suites/modules/server.mjs create mode 100644 dev-packages/node-integration-tests/suites/modules/test.ts diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts index 498c9b969ed9..8208cd603c98 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts @@ -123,4 +123,12 @@ test('Should capture an error and transaction for a app router page', async ({ p expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); expect(transactionEvent.tags?.['my-isolated-tag']).toBe(true); expect(transactionEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); + + // Modules are set for Next.js + expect(errorEvent.modules).toEqual( + expect.objectContaining({ + '@sentry/nextjs': expect.any(String), + '@playwright/test': expect.any(String), + }), + ); }); diff --git a/dev-packages/node-integration-tests/suites/modules/instrument.mjs b/dev-packages/node-integration-tests/suites/modules/instrument.mjs new file mode 100644 index 000000000000..9ffde125d498 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/modules/instrument.mjs @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/modules/server.js b/dev-packages/node-integration-tests/suites/modules/server.js new file mode 100644 index 000000000000..f537a31a4008 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/modules/server.js @@ -0,0 +1,22 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.get('/test1', (_req, _res) => { + throw new Error('error_1'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/modules/server.mjs b/dev-packages/node-integration-tests/suites/modules/server.mjs new file mode 100644 index 000000000000..ca47d79dd9c0 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/modules/server.mjs @@ -0,0 +1,13 @@ +import * as Sentry from '@sentry/node'; +import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import express from 'express'; + +const app = express(); + +app.get('/test1', (_req, _res) => { + throw new Error('error_1'); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/modules/test.ts b/dev-packages/node-integration-tests/suites/modules/test.ts new file mode 100644 index 000000000000..07bc238ec229 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/modules/test.ts @@ -0,0 +1,48 @@ +import { SDK_VERSION } from '@sentry/core'; +import { join } from 'path'; +import { afterAll, describe, expect, test } from 'vitest'; +import { cleanupChildProcesses, createRunner } from '../../utils/runner'; + +describe('modulesIntegration', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('CJS', async () => { + const runner = createRunner(__dirname, 'server.js') + .withMockSentryServer() + .expect({ + event: { + modules: { + // exact version comes from require.caches + express: '4.21.1', + // this comes from package.json + '@sentry/node': SDK_VERSION, + yargs: '^16.2.0', + }, + }, + }) + .start(); + runner.makeRequest('get', '/test1', { expectError: true }); + await runner.completed(); + }); + + test('ESM', async () => { + const runner = createRunner(__dirname, 'server.mjs') + .withInstrument(join(__dirname, 'instrument.mjs')) + .withMockSentryServer() + .expect({ + event: { + modules: { + // this comes from package.json + express: '^4.21.1', + '@sentry/node': SDK_VERSION, + yargs: '^16.2.0', + }, + }, + }) + .start(); + runner.makeRequest('get', '/test1', { expectError: true }); + await runner.completed(); + }); +}); diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 322f2e320624..8898b3495ba9 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -410,6 +410,14 @@ export function constructWebpackConfigFunction( ); } + // We inject a map of dependencies that the nextjs app has, as we cannot reliably extract them at runtime, sadly + newConfig.plugins = newConfig.plugins || []; + newConfig.plugins.push( + new buildContext.webpack.DefinePlugin({ + __SENTRY_SERVER_MODULES__: JSON.stringify(_getModules(projectDir)), + }), + ); + return newConfig; }; } @@ -825,3 +833,21 @@ function addOtelWarningIgnoreRule(newConfig: WebpackConfigObjectWithModuleRules) newConfig.ignoreWarnings.push(...ignoreRules); } } + +function _getModules(projectDir: string): Record { + try { + const packageJson = path.join(projectDir, 'package.json'); + const packageJsonContent = fs.readFileSync(packageJson, 'utf8'); + const packageJsonObject = JSON.parse(packageJsonContent) as { + dependencies?: Record; + devDependencies?: Record; + }; + + return { + ...packageJsonObject.dependencies, + ...packageJsonObject.devDependencies, + }; + } catch { + return {}; + } +} diff --git a/packages/node/src/integrations/modules.ts b/packages/node/src/integrations/modules.ts index e15aa9dd245b..f4a87a749766 100644 --- a/packages/node/src/integrations/modules.ts +++ b/packages/node/src/integrations/modules.ts @@ -1,26 +1,23 @@ import { existsSync, readFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import type { IntegrationFn } from '@sentry/core'; -import { defineIntegration, logger } from '@sentry/core'; -import { DEBUG_BUILD } from '../debug-build'; +import { defineIntegration } from '@sentry/core'; import { isCjs } from '../utils/commonjs'; -let moduleCache: { [key: string]: string }; +type ModuleInfo = Record; + +let moduleCache: ModuleInfo | undefined; const INTEGRATION_NAME = 'Modules'; -const _modulesIntegration = (() => { - // This integration only works in CJS contexts - if (!isCjs()) { - DEBUG_BUILD && - logger.warn( - 'modulesIntegration only works in CommonJS (CJS) environments. Remove this integration if you are using ESM.', - ); - return { - name: INTEGRATION_NAME, - }; - } +declare const __SENTRY_SERVER_MODULES__: Record; + +/** + * This is replaced at build time with the modules loaded by the server. + */ +const SERVER_MODULES = typeof __SENTRY_SERVER_MODULES__ === 'undefined' ? {} : __SENTRY_SERVER_MODULES__; +const _modulesIntegration = (() => { return { name: INTEGRATION_NAME, processEvent(event) { @@ -36,13 +33,14 @@ const _modulesIntegration = (() => { /** * Add node modules / packages to the event. - * - * Only works in CommonJS (CJS) environments. + * For this, multiple sources are used: + * - They can be injected at build time into the __SENTRY_SERVER_MODULES__ variable (e.g. in Next.js) + * - They are extracted from the dependencies & devDependencies in the package.json file + * - They are extracted from the require.cache (CJS only) */ export const modulesIntegration = defineIntegration(_modulesIntegration); -/** Extract information about paths */ -function getPaths(): string[] { +function getRequireCachePaths(): string[] { try { return require.cache ? Object.keys(require.cache as Record) : []; } catch (e) { @@ -51,17 +49,23 @@ function getPaths(): string[] { } /** Extract information about package.json modules */ -function collectModules(): { - [name: string]: string; -} { +function collectModules(): ModuleInfo { + return { + ...SERVER_MODULES, + ...getModulesFromPackageJson(), + ...(isCjs() ? collectRequireModules() : {}), + }; +} + +/** Extract information about package.json modules from require.cache */ +function collectRequireModules(): ModuleInfo { const mainPaths = require.main?.paths || []; - const paths = getPaths(); - const infos: { - [name: string]: string; - } = {}; - const seen: { - [path: string]: boolean; - } = {}; + const paths = getRequireCachePaths(); + + // We start with the modules from package.json (if possible) + // These may be overwritten by more specific versions from the require.cache + const infos: ModuleInfo = {}; + const seen = new Set(); paths.forEach(path => { let dir = path; @@ -71,7 +75,7 @@ function collectModules(): { const orig = dir; dir = dirname(orig); - if (!dir || orig === dir || seen[orig]) { + if (!dir || orig === dir || seen.has(orig)) { return undefined; } if (mainPaths.indexOf(dir) < 0) { @@ -79,7 +83,7 @@ function collectModules(): { } const pkgfile = join(orig, 'package.json'); - seen[orig] = true; + seen.add(orig); if (!existsSync(pkgfile)) { return updir(); @@ -103,9 +107,34 @@ function collectModules(): { } /** Fetches the list of modules and the versions loaded by the entry file for your node.js app. */ -function _getModules(): { [key: string]: string } { +function _getModules(): ModuleInfo { if (!moduleCache) { moduleCache = collectModules(); } return moduleCache; } + +interface PackageJson { + dependencies?: Record; + devDependencies?: Record; +} + +function getPackageJson(): PackageJson { + try { + const filePath = join(process.cwd(), 'package.json'); + const packageJson = JSON.parse(readFileSync(filePath, 'utf8')) as PackageJson; + + return packageJson; + } catch (e) { + return {}; + } +} + +function getModulesFromPackageJson(): ModuleInfo { + const packageJson = getPackageJson(); + + return { + ...packageJson.dependencies, + ...packageJson.devDependencies, + }; +} diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index 1536242cfdcb..e693d3976fe4 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -40,10 +40,6 @@ import { defaultStackParser, getSentryRelease } from './api'; import { NodeClient } from './client'; import { initOpenTelemetry, maybeInitializeEsmLoader } from './initOtel'; -function getCjsOnlyIntegrations(): Integration[] { - return isCjs() ? [modulesIntegration()] : []; -} - /** * Get default integrations, excluding performance. */ @@ -69,7 +65,7 @@ export function getDefaultIntegrationsWithoutPerformance(): Integration[] { nodeContextIntegration(), childProcessIntegration(), processSessionIntegration(), - ...getCjsOnlyIntegrations(), + modulesIntegration(), ]; } From 3e999fd4c8411564a67d7d77330f9e8b9449f0cb Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 12 Jun 2025 14:39:10 +0200 Subject: [PATCH 2/3] linting fixes --- dev-packages/node-integration-tests/suites/modules/server.js | 2 +- dev-packages/node-integration-tests/suites/modules/server.mjs | 2 +- dev-packages/node-integration-tests/suites/modules/test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/modules/server.js b/dev-packages/node-integration-tests/suites/modules/server.js index f537a31a4008..9b24c0845ac0 100644 --- a/dev-packages/node-integration-tests/suites/modules/server.js +++ b/dev-packages/node-integration-tests/suites/modules/server.js @@ -13,7 +13,7 @@ const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node const app = express(); -app.get('/test1', (_req, _res) => { +app.get('/test1', () => { throw new Error('error_1'); }); diff --git a/dev-packages/node-integration-tests/suites/modules/server.mjs b/dev-packages/node-integration-tests/suites/modules/server.mjs index ca47d79dd9c0..6edeb78c703f 100644 --- a/dev-packages/node-integration-tests/suites/modules/server.mjs +++ b/dev-packages/node-integration-tests/suites/modules/server.mjs @@ -4,7 +4,7 @@ import express from 'express'; const app = express(); -app.get('/test1', (_req, _res) => { +app.get('/test1', () => { throw new Error('error_1'); }); diff --git a/dev-packages/node-integration-tests/suites/modules/test.ts b/dev-packages/node-integration-tests/suites/modules/test.ts index 07bc238ec229..89fe98c62867 100644 --- a/dev-packages/node-integration-tests/suites/modules/test.ts +++ b/dev-packages/node-integration-tests/suites/modules/test.ts @@ -1,6 +1,6 @@ import { SDK_VERSION } from '@sentry/core'; import { join } from 'path'; -import { afterAll, describe, expect, test } from 'vitest'; +import { afterAll, describe, test } from 'vitest'; import { cleanupChildProcesses, createRunner } from '../../utils/runner'; describe('modulesIntegration', () => { From cec19a7f708053a78e815a1064196ddd3a597984 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 12 Jun 2025 17:17:40 +0200 Subject: [PATCH 3/3] better comment --- packages/node/src/integrations/modules.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/node/src/integrations/modules.ts b/packages/node/src/integrations/modules.ts index f4a87a749766..50f3a3b3aa8d 100644 --- a/packages/node/src/integrations/modules.ts +++ b/packages/node/src/integrations/modules.ts @@ -13,7 +13,8 @@ const INTEGRATION_NAME = 'Modules'; declare const __SENTRY_SERVER_MODULES__: Record; /** - * This is replaced at build time with the modules loaded by the server. + * `__SENTRY_SERVER_MODULES__` can be replaced at build time with the modules loaded by the server. + * Right now, we leverage this in Next.js to circumvent the problem that we do not get access to these things at runtime. */ const SERVER_MODULES = typeof __SENTRY_SERVER_MODULES__ === 'undefined' ? {} : __SENTRY_SERVER_MODULES__;

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!