sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

@sentry/nextjs build fails on Yarn PnP mode

Open artechventure opened this issue 1 year ago • 8 comments

Is there an existing issue for this?

  • [x] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
  • [x] I have reviewed the documentation https://docs.sentry.io/
  • [x] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

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.

artechventure avatar Sep 10 '24 07:09 artechventure

hi @artechventure, which yarn version are you using?

chargome avatar Sep 10 '24 08:09 chargome

hi @artechventure, which yarn version are you using?

Hi, I'm using 4.0.2

artechventure avatar Sep 10 '24 08:09 artechventure

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!

chargome avatar Sep 10 '24 08:09 chargome

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

wwarby avatar Oct 10 '24 09:10 wwarby

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

chargome avatar Oct 11 '24 07:10 chargome

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/

OllieJennings avatar Oct 14 '24 11:10 OllieJennings

@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

wwarby avatar Oct 14 '24 16:10 wwarby

I'm using this for now:

Hotfix

.yarn/patches/@sentry-utils-npm-8.34.0-6a5785e49c.patch:

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:

{
  ...,

  "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:

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.)

lesderid avatar Oct 15 '24 12:10 lesderid

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.

icopp avatar Oct 29 '24 21:10 icopp

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

chargome avatar Nov 05 '24 13:11 chargome

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.

github-actions[bot] avatar Nov 05 '24 14:11 github-actions[bot]