Skip to content
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

Infinite error loop with modulesIntegration in ESM #14165

Closed
3 tasks done
kyranet opened this issue Nov 3, 2024 · 3 comments · Fixed by #14169
Closed
3 tasks done

Infinite error loop with modulesIntegration in ESM #14165

kyranet opened this issue Nov 3, 2024 · 3 comments · Fixed by #14169
Labels
Package: node Issues related to the Sentry Node SDK

Comments

@kyranet
Copy link

kyranet commented Nov 3, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

8.36.0

Framework Version

Node.js v22.11.0

Link to Sentry event

No response

Reproduction Example/SDK Setup

import * as Sentry from '@sentry/node';

Sentry.init({
	dsn: envParseString('SENTRY_URL'),
	integrations: [
		Sentry.consoleIntegration(),
		Sentry.functionToStringIntegration(),
		Sentry.linkedErrorsIntegration(),
		Sentry.modulesIntegration(),
		Sentry.onUncaughtExceptionIntegration(),
		Sentry.onUnhandledRejectionIntegration(),
		Sentry.httpIntegration({ breadcrumbs: true }),
		Sentry.prismaIntegration(),
		Sentry.rewriteFramesIntegration({ root: rootFolder })
	]
});

Steps to Reproduce

  1. Make an ESM-only Node.js application
  2. Setup the SDK as shown above
  3. Catch an exception (e.g. with captureException())
  4. Watch Sentry get into an infinite loop, using 100% CPU

Expected Result

Everything works as expected and an incident is created in the dashboard.

Actual Result

It doesn't create the incident... nor it responds, because:

The collectModules() function uses require even in the ESM version, where it's not defined: https://yarnpkg.com/package?q=%40sentry%2Fnode&name=%40sentry%2Fnode&file=%2Fbuild%2Fesm%2Fintegrations%2Fmodules.js

This causes whatever code is calling to throw an error over and over:

ReferenceError: require is not defined
    at collectModules (app\node_modules\@sentry\node\src\integrations\modules.ts:42:21)
    at _getModules (app\node_modules\@sentry\node\src\integrations\modules.ts:93:19)
    at Object.processEvent (app\node_modules\@sentry\node\src\integrations\modules.ts:16:12)
    at Object.assign.id (app\node_modules\@sentry\core\src\integration.ts:139:105)
    at <anonymous> (app\node_modules\@sentry\core\src\eventProcessors.ts:20:22)
    at new SyncPromise (app\node_modules\@sentry\utils\src\syncpromise.ts:59:7)
    at notifyEventProcessors (app\node_modules\@sentry\core\src\eventProcessors.ts:15:10)
    at <anonymous> (app\node_modules\@sentry\core\src\eventProcessors.ts:26:26)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)

Which is repeated indefinitely, ramping the CPU usage to 100% and making the application not respond.

Removing Sentry.modulesIntegration() from the integrations list resolves this bug.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 3, 2024
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Nov 3, 2024
@andreiborza
Copy link
Member

Hi @kyranet, thanks so much for investigating and filing this. Indeed we should add some guarding here.
Will add it to our backlog.

@AbhiPrasad
Copy link
Member

This is a duplicate of #12500.

Going to close this in favour of tracking this issue there.

@AbhiPrasad AbhiPrasad closed this as not planned Won't fix, can't repro, duplicate, stale Nov 4, 2024
AbhiPrasad added a commit that referenced this issue Nov 4, 2024
resolves #12500
resolves #14165

`modulesIntegration` uses top-level require which will crash ESM apps if
you explicitly import and use the integration.

```mjs
// index.mjs
Sentry.init({
    dsn: '__DSN__',
    integrations: [
        Sentry.modulesIntegration(),
    ]
});
```

This fixes that by adding a boolean check for cjs apps, and logging out
a warning as a result.
Copy link
Contributor

github-actions bot commented Nov 5, 2024

A PR closing this issue has just been released 🚀

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

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
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants