Repository navigation
Memory leak on Node 19/20/22 with includeLocalVariables enabled #16041
Description
Is there an existing issue for this?
- I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
- I have reviewed the documentation https://docs.sentry.io/
- I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases
How do you use Sentry?
Sentry Saas (sentry.io)
Which SDK are you using?
@sentry/node
SDK Version
9.1.0
Framework Version
9.1.0
Link to Sentry event
No response
Reproduction Example/SDK Setup
See #14162
Our instance of issue is summarised in this comment
Ours is a reasonably high traffic express/node.js app, and with includeLocalVariables native memory usage increases until OOM.
Steps to Reproduce
See #14162
Expected Result
No native memory leak when using includeLocalVariables
Actual Result
Native memory leak is present when using includeLocalVariables
Reacted by Jeff Zellner, Pablo Aldana and Calvin Lee
Reacted by Pablo Aldana
Activity
Lms24 commentedon Apr 14, 2025
Hey @tellisnz thanks for taking the time to report this!
I saw your comment in the other issue and I fully understand that this is not a priority for you. However, for us on our end, it's also hard to debug this without a reproduction or a memory profile.
I'll therefore add the
Waiting for: Communitylabel to this issue which will auto-close the issue unless someone replies with a reproduction.EDIT: fixed comment link
tellisnz commentedon Apr 14, 2025
obviously not going to give a native memory dump.
I understand the difficulty in being able to figure this out without a reproduction, but the lack of effort in attempting to find it yourselves given clear evidence of an issue is surprising.
not sure why I was asked to create this if this was going to be the response.
Lms24 commentedon Apr 14, 2025
Again, I fully understand that this is frustrating, so I want to share some more context:
We already tried reproducing this in #14162 where the user originally even shared a reproduction attempt. Unfortunately, we could not reproduce it on our end. We asked for a memory profile or heap snapshot but did not receive one.
I will keep this open for now so that we can track it.
29 remaining items
ericallam commentedon Aug 13, 2025
@Lms24 we're seeing the same thing on https://github.com/triggerdotdev/trigger.dev using
@sentry/remix9.4.0.I wrote a memory profiling harness to try and track down the leak that:
I then look at the last two snapshots and use "Comparison" mode in Chrome devtools.
My first finding is that with SENTRY_DSN not set there is almost 0 memory growth between the two snapshots.
My second finding is that when
includeLocalVariables: true(or left to the default oftrue), we can see sizable positive deltas between the two snapshots:And here is the same view of the two snapshots with
includeLocalVariables: false:As you can see this eliminates most of the growth. Happy to share my v8 snapshots
ericallam commentedon Aug 13, 2025
@Lms24 https://drive.google.com/file/d/1n9XzButYbLxWdztuAgEH8NK6iC9kSNY-/view?usp=drive_link
tonynelson19 commentedon Aug 13, 2025
Are you sure this is the case? From what I can see, the
includeLocalVariablesoption isundefinedby default.ericallam commentedon Aug 13, 2025
Yea I'm not actually sure yet if it was
includeLocalVariables. I think actually what I was seeing fix the issue was going from@sentry/remix9.40.0 to9.46.0, but am confirming that nowericallam commentedon Aug 13, 2025
Yea okay you can ignore all of the above, I think my memory test harness needs some improvement before I can say definitely what is going on
ericallam commentedon Aug 13, 2025
ericallam commentedon Aug 14, 2025
@Lms24 okay I managed to track down the actual issue. It's the call to
request.clone()here:sentry-javascript/packages/remix/src/vendor/instrumentation.ts
Line 289 in f25664b
Removing the call to
clone()there stops the memory leak from happening. It seems like it might possibly be related to this undici issue but I can't be sure.This is in a Remix 2.1.0 app on Node.js v20.11.1. I'm just going to
pnpm patchthe@sentry/remixpackage to get this to stop leaking for us in the meantime. I'll include the PR for that here when it's ready.Lms24 commentedon Aug 14, 2025
Thanks for continuing to look into this @ericallam ! I remember removing the
clonecall in Sveltekit a while ago (though for a different reason) in #15368. Maybe we can do the same thing in Remix.The reason why we clone the request in the first place was to avoid interacting with the original request when we consume the request. Though I think that was only a problem with the request body. So maybe it's actually fine to just remove it.
Already brought this up within our team to discuss it further.
ericallam commentedon Aug 14, 2025
@Lms24 yea you do consume the body with the call to the
request.formData(), which we are removing with our patch.ericallam commentedon Aug 14, 2025
@Lms24 did some more experimentation with this and discovered that the leak only occurs when the request is cloned AND the cloned request's body is not consumed. My guess is that the
await request.formData()doesn't actually consume the body stream if the request body isapplication/json, which is what I was sending to the server to test this all out.ericallam commentedon Aug 14, 2025
Here's our PR: triggerdotdev/trigger.dev#2389
md384 commentedon Aug 14, 2025
Similarly to the original issue (node/express), we have seen memory issues when
includeLocalVariablesis set to true.For us, we observed the behavior when upgrading the JS SDK from 8.30 to 8.55 on node 22. Node 18 was fine on 8.55.
We have also observed that disabling the http node integration also stops the memory leak so there may be an interaction between that and the
includeLocalVariablesfeature.