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

ref(core): Log normalizeDepth when normalization is skipped #4574

Merged

Conversation

lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented Feb 15, 2022

Continuing debugging of #2809. In the most recent example event there, it's clear from the stringifying error's stacktrace that the event is hitting BaseClient._processEvent, and we can tell from the line number in that stack trace that it's getting past _prepareEvent, which is what calls normalize.

In _prepareEvent, the only way for normalization not to run is if the normalizeDepth option either isn't a number or is set to a value <= 0. (It's got a default value, so it not being set directly by the user isn't the issue.)

if (typeof normalizeDepth === 'number' && normalizeDepth > 0) {
return this._normalizeEvent(evt, normalizeDepth);
}

In order to better diagnose what the problem might be, this PR adds logging of the normalizeDepth value in cases where an event fails to be normalized when it should.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 15, 2022

size-limit report

Path Base Size (b46674c) Current Size Change
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 19.65 KB 19.68 KB +0.16% 🔺
@sentry/browser - ES5 CDN Bundle (minified) 63 KB 63.17 KB +0.28% 🔺
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.32 KB 18.35 KB +0.18% 🔺
@sentry/browser - ES6 CDN Bundle (minified) 56.15 KB 56.35 KB +0.36% 🔺
@sentry/browser - Webpack (gzipped + minified) 22.1 KB 22.13 KB +0.14% 🔺
@sentry/browser - Webpack (minified) 75.86 KB 76.07 KB +0.27% 🔺
@sentry/react - Webpack (gzipped + minified) 22.14 KB 22.17 KB +0.14% 🔺
@sentry/nextjs Client - Webpack (gzipped + minified) 46.26 KB 46.31 KB +0.1% 🔺
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 27.1 KB 27.13 KB +0.11% 🔺

@lobsterkatie lobsterkatie force-pushed the kmclb-log-normalizeDepth-when-normalization-skipped branch from 3c21b90 to 7d0463d Compare February 15, 2022 00:59
@lobsterkatie lobsterkatie force-pushed the kmclb-log-normalizeDepth-when-normalization-skipped branch from 7d0463d to f123207 Compare February 15, 2022 02:22
@lobsterkatie lobsterkatie merged commit 00d359d into master Feb 15, 2022
@lobsterkatie lobsterkatie deleted the kmclb-log-normalizeDepth-when-normalization-skipped branch February 15, 2022 02:47
lobsterkatie added a commit that referenced this pull request Feb 15, 2022
lobsterkatie added a commit that referenced this pull request Apr 6, 2022
…4870)

This is a follow up to #4425 and #4574, all in aid of debugging #2809, wherein serializing events leads to a circular reference error. One of the possible culprits is that the event is somehow skipping our usual normalization process, and one of the reasons that might happen is if `normalizeDepth` is set to something other than a number. We're already logging its value in problematic cases, and this change makes it so that we'll now log its type as well.
lobsterkatie added a commit that referenced this pull request Jun 16, 2022
In #4425 and #4574, various hacks were added to the SDK in aid of debugging #2809. Now that that issue is closed, they (and the clean-up code in tests which was there to compensate for them) can come out.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants