Default depth for ExtraErrorData not applied

[rchl]

• Review the documentation: https://docs.sentry.io/
• Search for existing issues: https://github.com/getsentry/sentry-javascript/issues
• Use the latest release: https://github.com/getsentry/sentry-javascript/releases
• Provide a link to the affected event from your Sentry account

Package + Version

• @sentry/browser
• @sentry/node
• raven-js
• raven-node (raven for node)
• other:

Version:

6.14.1

Description

I have just finished investigating some nasty issue where triggering an error on the server side resulted in 100% CPU usage and eventually getting OOM.

The infinite loop triggered from the ExtraErrorData integration, when it tried to normalize an axios error using normalize function from

sentry-javascript/packages/utils/src/object.ts

Line 342 in 4887f98

return JSON.parse(JSON.stringify(input, (key: string, value: any) => walk(key, value, depth)));

Turns out the issue only manifests itself when also having dd-trace enabled in the service but that's not necessarily the root issue. The issue for me was that I have passed an empty object to the ExtraErrorData constructor:

sentry-javascript/packages/integrations/src/extraerrordata.ts

Line 24 in 4887f98

public constructor(private readonly _options: ExtraErrorDataOptions = { depth: 3 }) {}

That resulted in depth being undefined rather than defaulting to 3 and this trigged this infinite loop.

Now, I'm not sure what's the main issue here:

• the fact that depth doesn't default to 3 when empty object is provided? This integration only accepts the depth option but if it would support more options, would it make sense to use default value for options that are not provided? I think it would, so it probably also makes sense to do that when only one option is supported.
• why does it go into an infinite loop when dd-trace is enabled but not otherwise? Shouldn't it break the cycle in any case? I haven't investigated what exactly makes it go into an infinite loop but must be something that dd-trace does with the Request/Response objects that the axios error contains.

[AbhiPrasad]

Thanks for raising an issue. We can probably change _options to use a default value, PRs and contributions are welcome to help out here!

why does it go into an infinite loop when dd-trace is enabled but not otherwise? Shouldn't it break the cycle in any case? I haven't investigated what exactly makes it go into an infinite loop but must be something that dd-trace does with the Request/Response objects that the axios error contains

dd-trace might be patching the objects, which might be causing circular references?

[rchl]

dd-trace might be patching the objects, which might be causing circular references?

Yes but the code is supposed to be equipped to handle that (the memo argument):

sentry-javascript/packages/utils/src/object.ts

Line 280 in 4887f98

export function walk(key: string, value: any, depth: number = +Infinity, memo: Memo = new Memo()): any {

[AbhiPrasad]

Might be something where the reference equality doesn't match, hence the weakset lookup never works - not sure, just brainstorming.

We don't have the bandwidth to actively investigate this atm, but I think asking the DataDog folks is probably the next best course of action. If they come back with something that's on our end, let us know and we can take a look.