Skip to content

@sentry/tracing does not work with modern yarn (in PnP mode) #4076

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.

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

Closed
2 tasks done
andreialecu opened this issue Oct 19, 2021 · 2 comments
Closed
2 tasks done

@sentry/tracing does not work with modern yarn (in PnP mode) #4076

andreialecu opened this issue Oct 19, 2021 · 2 comments
Labels
Package: node Issues related to the Sentry Node SDK Type: Improvement

Comments

@andreialecu
Copy link
Contributor

andreialecu commented Oct 19, 2021

Package + Version

  • @sentry/node: 6.13.3
  • @sentry/tracing: 6.13.3

Version:

6.13.3

Description

While trying to add tracing instrumentation for mongoose in a nodejs app that uses Yarn 3.0 in PnP mode I noticed that nothing was happening, and tracing was silently failing to do anything.

The problem is that PnP mode is strict about which dependencies are allowed to be required, and a package that doesn't define the dep in its package.json (dependencies or peerDependencies) will not be able to require it.

See: https://yarnpkg.com/advanced/rulebook/#packages-should-only-ever-require-what-they-formally-list-in-their-dependencies

Neither @sentry/tracing or @sentry/utils list mongoose/mongodb/pg/mysql as dependencies.

The reason this fails is:

const moduleName = this._useMongoose ? 'mongoose' : 'mongodb';
const pkg = loadModule<{ Collection: MongoCollection }>(moduleName);

Which then delegates to:

try {
mod = dynamicRequire(module, moduleName);
} catch (e) {
// no-empty
}

That empty catch actually suppresses this error:

Error: @sentry/utils tried to access mongoose (a peer dependency) but it isn't provided by its ancestors; this makes the require call ambiguous and unsound.

Required package: mongoose
Required by: @sentry/utils@virtual:...

Ancestor breaking the chain: @sentry/core@npm:6.13.3
Ancestor breaking the chain: @sentry/hub@npm:6.13.3
Ancestor breaking the chain: @sentry/node@npm:6.13.3

This is going to be tricky to fix because of the long chain of dependencies needing to pass the target dependency along properly.

One possible suggestion to get around this would be to:

The caller might then do something like:

const MongoTracingIntegration = new Tracing.Integrations.Mongo({
  collection: require("mongoose").Collection,
});

This would bypass the require call being done by the sentry library, and delegate it to the user instead, providing a way around the problem.

Something similar would need to be done with all the other integrations.

Slightly related: #3172 (different issue on PnP)

andreialecu added a commit to andreialecu/sentry-javascript that referenced this issue Oct 19, 2021
Fixes getsentry#4076

Uses `createRequire` to ensure that modules are correctly located
@andreialecu
Copy link
Contributor Author

Found a simpler solution that doesn't require any changes to the API and fixed this in #4077

andreialecu added a commit to andreialecu/sentry-javascript that referenced this issue Oct 19, 2021
Fixes getsentry#4076

Uses `createRequire` to ensure that modules are correctly located
andreialecu added a commit to andreialecu/sentry-javascript that referenced this issue Oct 19, 2021
@iker-barriocanal iker-barriocanal added Package: node Issues related to the Sentry Node SDK Type: Improvement and removed Status: Untriaged labels Oct 20, 2021
@github-actions
Copy link
Contributor

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: node Issues related to the Sentry Node SDK Type: Improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants