Remove the dynamicRequire hack to fix scope memory leak

[Madumo]

That bug has been discussed in #1762. When bundling @sentry/node with webpack, it causes a sentry to leak memory.

The problem resides in the dynamicRequire function. This function expects the module.require function to exist, but when bundled with webpack it doesn't. When we use it to require the domain module, which is needed to clone the scope instance, it throws an error and sentry fallback to using the global scope instance.

Each incoming requests then use the same scope instance and attach a new event processor callback to it. Since those event processors are never unassigned and depend on the whole scope object to be garbage collected, their closure which capture a lot of stuff (like the request object) stays in memory.

Recap

dynamicRequire is used to load the domain module

mod.require is undefined

What the mod object looks like

dynamicRequire is making the assumption that the module is the one from node, but when bundled by webpack it's an object provided by webpack that doesn't have the Module __proto__.

dynamicRequire throws an error since mod.require is undefined and getHubFromActiveDomain end up using getHubFromCarrier(registry) instead of getHubFromCarrier(activeDomain)

I guess this might also cause bugs when processing an error since all the event processors previously added by previous requests will be called? 🤔

To fix this issue I replaced dynamicRequire with require, and it now works, no more leak! 🎉
I guess the dynamicRequire hack was only needed with older versions of webpack and now isn't needed anymore.

[HazAT]

ref: getsentry/sentry-electron#92

[kamilogorek]

JS tooling is hard. You never know when something changes and breaks your own workaround ¯_(ツ)_/¯
Thanks for investigating @Madumo!

[Madumo]

@kamilogorek You're right, even more so when it's about js modules. The ecosystem is in a weird state right now between what is official spec and what is invented userland legacy stuff, what is supported, and how different bundlers decides ton interpret and implement it. It's a goddamn mess, but we'll get to a point where it's all cleared up someday... I hope 😅

Thanks for merging and releasing this so quickly ✌️

[kamilogorek]

@Madumo do you have your test cases still laying around by any chance? Unfortunately, the builds are now failing for react-native, as bundler tries to optimize require('domain') and it fails to load the module...

If so, can you test out this silly change?:

const req = require;
const domain = req('domain');

It works around the react-native bundler correctly, but we need to be sure that it works fine with webpack as well.

[HazAT]

Ref: #2521

[Madumo]

Ha! Fixed webpack, broke metro. Dammit. I'll check!

[Madumo]

I patched it manually in my node_modules, made a production build, and tried it. It seems to work fine!