Memory leak on Node 19/20/22 with includeLocalVariables enabled

[tellisnz]

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?

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 https://github.com/getsentry/sentry-javascript/issues/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 https://github.com/getsentry/sentry-javascript/issues/14162

Expected Result

No native memory leak when using includeLocalVariables

Actual Result

Native memory leak is present when using includeLocalVariables

[Lms24]

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: Community label to this issue which will auto-close the issue unless someone replies with a reproduction.

EDIT: fixed comment link

[tellisnz]

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]

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.

[timfish]

I've spent a couple of days looking into issues around memory leaks and includeLocalVariables but have so far been unable to reproduce anything locally.

Native memory leak is present when using includeLocalVariables

What do you mean by "native memory leak"? The leak isn't in the JavaScript allocations?

[designblooz]

I also ran into this issue, after upgrading @nestjs/sentry from 8.30.0 to 9.17.0 a lot of my nestjs services restarted due to OOM. After disabling includeLocalVariables, memory usage is stable.

[getsantry[bot]]

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 remove the label Waiting for: Community, I will leave it alone ... forever!

---

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

[whaagmans]

I also noticed, after upgrading Sentry from 7.81.1 to 8.41.0, that my Express APIs' memory usage consistently increased until they were either manually restarted or OOM. After seeing this issue and setting includeLocalVariables to false. The memory is indeed stable. So, it does appear to be correlated with this feature.

Sidenote: I have yet to upgrade to the latest available version, so hopefully it is resolved there, but have not seen any mention of this in the changelogs so far.

[getsantry[bot]]

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 remove the label Waiting for: Community, I will leave it alone ... forever!

---

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

[ericallam]

@Lms24 we're seeing the same thing on https://github.com/triggerdotdev/trigger.dev using @sentry/remix 9.4.0.

I wrote a memory profiling harness to try and track down the leak that:

• Does a fresh build
• Runs our web server with SENTRY_DSN either set or not
• Executes 10 requests
• Takes an initial v8 snapshot
• Executes 1000 requests
• Takes another v8 snapshot
• Executes another 1000 requests
• Takes another v8 snapshot

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 of true), 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]

@Lms24 https://drive.google.com/file/d/1n9XzButYbLxWdztuAgEH8NK6iC9kSNY-/view?usp=drive_link

[tonynelson19]

or left to the default of true

Are you sure this is the case? From what I can see, the includeLocalVariables option is undefined by default.

[ericallam]

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/remix 9.40.0 to 9.46.0, but am confirming that now

[ericallam]

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]

[ericallam]

@Lms24 okay I managed to track down the actual issue. It's the call to request.clone() here:

https://github.com/getsentry/sentry-javascript/blob/f25664bd6b2da4c99d44a6e639b38550a0e0d436/packages/remix/src/vendor/instrumentation.ts#L289

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 patch the @sentry/remix package to get this to stop leaking for us in the meantime. I'll include the PR for that here when it's ready.

[Lms24]

Thanks for continuing to look into this @ericallam ! I remember removing the clone call in Sveltekit a while ago (though for a different reason) in https://github.com/getsentry/sentry-javascript/pull/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]

@Lms24 yea you do consume the body with the call to the request.formData(), which we are removing with our patch.

[ericallam]

@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 is application/json, which is what I was sending to the server to test this all out.

[ericallam]

Here's our PR: https://github.com/triggerdotdev/trigger.dev/pull/2389

[md384]

Similarly to the original issue (node/express), we have seen memory issues when includeLocalVariables is 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 includeLocalVariables feature.