getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.99k stars 1.57k forks source link

@sentry/nextjs build fails on Yarn PnP mode #13641

Closed artechventure closed 5 days ago

artechventure commented 2 months ago

Is there an existing issue for this?

How do you use Sentry?

Self-hosted/on-premise

Which SDK are you using?

@sentry/nextjs

SDK Version

8.29.0

Framework Version

Next 14.2.5

Link to Sentry event

No response

Reproduction Example/SDK Setup

https://github.com/getsentry/sentry-javascript/blob/0d79b51e7ecbe26b1b235ed92410d2d07febdcde/packages/nextjs/src/config/webpack.ts#L335

Above part changed 6 months ago, I believe it fails on Yarn PnP since then.

https://github.com/getsentry/sentry-javascript/blob/0d79b51e7ecbe26b1b235ed92410d2d07febdcde/packages/utils/src/node.ts#L57

Inside function loadmodule, node_modules is hard-coded which will always returns undefined

Steps to Reproduce

  1. Setup yarn pnp workspace
  2. Import withSentryConfig from @sentry/nextjs inside next.config.js
  3. Run next build

Expected Result

Build success

Actual Result

TypeError: Cannot destructure property 'sentryWebpackPlugin' of 'utils.loadModule(...)' as it is undefined.

chargome commented 2 months ago

hi @artechventure, which yarn version are you using?

artechventure commented 2 months ago

hi @artechventure, which yarn version are you using?

Hi, I'm using 4.0.2

chargome commented 2 months ago

I was able to reproduce it, thanks for pointing that out. We'll put in on our backlog. Meanwhile you can revert to any yarn node-linker that uses node_modules, sorry for the inconvenience!

wwarby commented 1 month ago

Im seeing this error with nodeLinker: pnpm - is that expected? We really don't want to have t o use node-modules linker if possible, this seems to be our only issue switching to pnpm

chargome commented 1 month ago

@wwarby yeah I need to re-visit this. Are you using pnpm workspaces?

OllieJennings commented 3 weeks ago

I am encountering this issue on a TurboRepo mono-repo (npm workspaces) FYI, having to downgrade back to Sentry v7.

Can confirm that its because of this line:

sentry-javascript/packages/utils/src/node.ts

Line 57 in 0d79b51

mod = dynamicRequire(module, `${cwd()}/node_modules/${moduleName}`) as T;

Since npm has hoisted the package to the top of the workspace /node_modules/, however sentry is looking in the current path of the subpackage/node_moduels/

wwarby commented 3 weeks ago

@wwarby yeah I need to re-visit this. Are you using pnpm workspaces?

I don't think so. We're just using plain Yarn with this in our .yarnrc.yml file. The build works if I change nodeLinker to node-modules.

nodeLinker: pnpm

enableGlobalCache: false

yarnPath: .yarn/releases/yarn-4.5.0.cjs
lesderid commented 3 weeks ago

I'm using this for now:

Hotfix `.yarn/patches/@sentry-utils-npm-8.34.0-6a5785e49c.patch`: ```diff diff --git a/build/cjs/node.js b/build/cjs/node.js index 42c17acce0d6d4451eb5ece7f9aaf380d1be4cc9..cbf9c0c7f8efbeb2641fae9654c91b0b4e6904a9 100644 --- a/build/cjs/node.js +++ b/build/cjs/node.js @@ -49,17 +49,33 @@ function dynamicRequire(mod, request) { function loadModule(moduleName) { let mod; - try { - mod = dynamicRequire(module, moduleName); - } catch (e) { - // no-empty + const isYarnPnP = typeof require('pnpapi') !== 'undefined'; + + if (isYarnPnP) { + try { + const pnp = require('pnpapi'); + const modulePath = pnp.resolveRequest(moduleName, __filename); + if (modulePath) { + mod = dynamicRequire(module, modulePath); + } + } catch (e) { + console.error(`Failed to load module ${moduleName} with Yarn PnP`, e); + } } - try { - const { cwd } = dynamicRequire(module, 'process'); - mod = dynamicRequire(module, `${cwd()}/node_modules/${moduleName}`) ; - } catch (e) { - // no-empty + if (!mod) { + try { + mod = dynamicRequire(module, moduleName); + } catch (e) { + // no-empty + } + + try { + const { cwd } = dynamicRequire(module, 'process'); + mod = dynamicRequire(module, `${cwd()}/node_modules/${moduleName}`) ; + } catch (e) { + // no-empty + } } return mod; diff --git a/build/esm/node.js b/build/esm/node.js index 56936e06bb64149d1ed4fcca75c47135adaa228d..3fddc6e69e2b1c695268c88940c2b6d8cfe96873 100644 --- a/build/esm/node.js +++ b/build/esm/node.js @@ -47,17 +47,33 @@ function dynamicRequire(mod, request) { function loadModule(moduleName) { let mod; - try { - mod = dynamicRequire(module, moduleName); - } catch (e) { - // no-empty + const isYarnPnP = typeof require('pnpapi') !== 'undefined'; + + if (isYarnPnP) { + try { + const pnp = require('pnpapi'); + const modulePath = pnp.resolveRequest(moduleName, __filename); + if (modulePath) { + mod = dynamicRequire(module, modulePath); + } + } catch (e) { + console.error(`Failed to load module ${moduleName} with Yarn PnP`, e); + } } - try { - const { cwd } = dynamicRequire(module, 'process'); - mod = dynamicRequire(module, `${cwd()}/node_modules/${moduleName}`) ; - } catch (e) { - // no-empty + if (!mod) { + try { + mod = dynamicRequire(module, moduleName); + } catch (e) { + // no-empty + } + + try { + const { cwd } = dynamicRequire(module, 'process'); + mod = dynamicRequire(module, `${cwd()}/node_modules/${moduleName}`) ; + } catch (e) { + // no-empty + } } return mod; ``` `package.json`: ```json { ..., "resolutions": { "@sentry/utils@npm:8.34.0": "patch:@sentry/utils@npm%3A8.34.0#~/.yarn/patches/@sentry-utils-npm-8.34.0-6a5785e49c.patch" } } ``` `.yarnrc.yaml`: ```yaml packageExtensions: "@sentry/utils@*": dependencies: "@sentry/webpack-plugin": "*" ```

Edit: This is not a real solution, as it requires @sentry/utils to have a dependency on @sentry/webpack-plugin. (And it was generated by ChatGPT.)

icopp commented 1 week ago

I am encountering this issue on a TurboRepo mono-repo (npm workspaces) FYI, having to downgrade back to Sentry v7.

Can confirm that its because of this line:

sentry-javascript/packages/utils/src/node.ts

Line 57 in 0d79b51

mod = dynamicRequire(module, ${cwd()}/node_modules/${moduleName}) as T; Since npm has hoisted the package to the top of the workspace /node_modules/, however sentry is looking in the current path of the subpackage/node_moduels/

Same here. No pnpm or Yarn, just plain npm workspaces.

chargome commented 5 days ago

Once this is released there will be an automated comment in this issue. Please leave a message here if you still encounter problems afterwards!

github-actions[bot] commented 5 days ago

A PR closing this issue has just been released 🚀

This issue was referenced by PR #13751, which was included in the 8.37.0 release.