Skip to content

fix(node): Read module versions from the pnpm isolated layout #25526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JPeer264 wants to merge 1 commit into from
+57 −2
Open

fix(node): Read module versions from the pnpm isolated layout #25526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions dev-packages/node-integration-tests/suites/modules/scenario-isolated.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
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,
});

// `test.ts` writes these packages in the folders where the isolated layouts of pnpm and Bun keep them
require('./node_modules/.pnpm/pnpm-isolated-package@1.2.3/node_modules/pnpm-isolated-package');
require('./node_modules/.bun/bun-isolated-package@4.5.6/node_modules/bun-isolated-package');

Sentry.captureException(new Error('error_1'));
23 changes: 23 additions & 0 deletions dev-packages/node-integration-tests/suites/modules/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { SDK_VERSION } from '@sentry/core';
import { mkdirSync, rmSync, writeFileSync } from 'fs';
import { join } from 'path';
import { afterAll, describe, expect, test } from 'vitest';
import { cleanupChildProcesses, createRunner } from '../../utils/runner';
Expand Down Expand Up @@ -27,6 +28,28 @@ describe('modulesIntegration', () => {
await runner.completed();
});

test('CJS with the isolated layouts of pnpm and Bun', async () => {
const nodeModulesDir = join(__dirname, 'node_modules');
for (const [storeDir, name, version] of [
['.pnpm', 'pnpm-isolated-package', '1.2.3'],
['.bun', 'bun-isolated-package', '4.5.6'],
] as const) {
const packageDir = join(nodeModulesDir, storeDir, `${name}@${version}`, 'node_modules', name);
mkdirSync(packageDir, { recursive: true });
writeFileSync(join(packageDir, 'package.json'), JSON.stringify({ name, version }));
writeFileSync(join(packageDir, 'index.js'), 'module.exports = {};');
}

try {
await createRunner(__dirname, 'scenario-isolated.js')
.expect({ event: { modules: { 'pnpm-isolated-package': '1.2.3', 'bun-isolated-package': '4.5.6' } } })
.start()
.completed();
} finally {
rmSync(nodeModulesDir, { recursive: true, force: true });
}
});

test('ESM', async () => {
const runner = createRunner(__dirname, 'server.mjs')
.withInstrument(join(__dirname, 'instrument.mjs'))
Expand Down
22 changes: 20 additions & 2 deletions packages/node/src/integrations/modules.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { existsSync, readFileSync } from 'node:fs';
import { dirname, join } from 'node:path';
import { basename, dirname, join } from 'node:path';
import { GLOBAL_OBJ, type IntegrationFn } from '@sentry/core';

type ModuleInfo = Record<string, string>;
Expand Down Expand Up @@ -73,6 +73,24 @@ function collectModules(): ModuleInfo {
};
}

/**
* Whether `dir` is a node_modules folder of the main module, or a folder
* `<that node_modules>/<.pnpm or .bun>/<package id>/node_modules`, where the isolated layouts of pnpm and Bun keep
* each package.
*/
function isMainModulesDir(dir: string, mainPaths: string[]): boolean {

@isaacs isaacs Oct 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For .../node_modules/@scope/name/index.js, the walk checks .../node_modules/@scope (not a modules folder), then .../node_modules with orig = .../@scope, finds no @scope/package.json, and moves on. So no scoped package is ever read from require.cache, in any layout. I confirmed this with real installs: @isaacs/cliui is missing from the npm, pnpm, and Bun results (it only shows up when the cwd package.json lists it, as a range). This is not a regression, but the PR's goal is to report installed versions for pnpm users, and many of their deps are scoped (@sentry/*, @opentelemetry/*, @aws-sdk/*, ...).

Suggestion (could be a follow-up, since it's a pre-existing issue, but I'd fix it now, why not): treat a scope folder as part of the modules folder. I tested this one-line change on the built output, and it fixes scoped packages in the hoisted, pnpm, and Bun layouts:

function isMainModulesDir(dir: string, mainPaths: string[]): boolean {
  if (basename(dir).startsWith('@')) {
    return isMainModulesDir(dirname(dir), mainPaths);
  }
  // ...
}

if (mainPaths.includes(dir)) {
return true;
}

const storeDir = dirname(dirname(dir));

@isaacs isaacs Oct 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pnpm/Bun store holds every version of every package, including transitive ones. Each store entry now counts as a "main" package, so infos[info.name] = info.version takes whichever version require.cache lists last. I reproduced this with a fake pnpm layout: the app depends on debug@4.0.0, and lib depends on debug@2.0.0.

require('debug'); require('lib');  -> { debug: '2.0.0', lib: '1.0.0' }
require('lib'); require('debug');  -> { lib: '1.0.0', debug: '4.0.0' }

Before this PR, pnpm users got the range from package.json (^4...), which was at least the right major. Now they can get a wrong version, and which one changes with load order. With npm, nested copies (node_modules/a/node_modules/b) are not in mainPaths, so the top-level (app-facing) copy always wins. The PR body says the result is "like with npm and Yarn", but here it differs.

Suggestion: when a name has more than one store version, prefer the one that <main node_modules>/<name> resolves to. For example, collect store hits per name, and on conflict compare realpathSync(join(mainModulesDir, name)) with the package folder. Or, simpler: never let a store entry overwrite a name that already came from a store entry that the top-level symlink points to.

return (
basename(dir) === 'node_modules' &&
['.pnpm', '.bun'].includes(basename(storeDir)) &&
mainPaths.includes(dirname(storeDir))
);
}

/** Extract information about package.json modules from require.cache */
function collectRequireModules(): ModuleInfo {
const mainPaths = require.main?.paths || [];
Expand All @@ -94,7 +112,7 @@ function collectRequireModules(): ModuleInfo {
if (!dir || orig === dir || seen.has(orig)) {
return undefined;
}
if (mainPaths.indexOf(dir) < 0) {
if (!isMainModulesDir(dir, mainPaths)) {
return updir();
}

Expand Down
Loading