fix(utils): Fix faulty references in dropUndefinedKeys

[lforst]

For a long time, our helper function dropUndefinedKeys (which might more accurately be named dropKeysWithUndefinedValues) had a bug: when presented with an object containing a circular reference, it was perfectly happy to follow that cycle and recurse infinitely right along with the object. This was fixed in #5163 using a memoizer - we keep a reference to each value we process, and if we hit one we've seen before, we stop recursing. No more infinite loops, yay!

Unfortunately, that change introduced a new bug. By design, dropUndefinedKeys doesn't mutate the object it's given, instead performing a deep copy which just skips over any keys whose value is undefined, eventually returning an object wholely separate from the original. If there's a circular reference in the original, its copy thus points from one spot in the new object to another spot in the new object and then back again. The cycle might be longer than two nodes, of course, but the overall point stands: every member of the cycle in the new object must itself be contained in (or equal to) the new object.

The problem with the fix in #5163 is that it violates that rule. Before that change, when dropUndefinedKeys recursed infinitely, it wasn't creating a circular reference in the new object. Instead, it was creating infinitely nested copies of the members of the cycle. In other words, when presented with

hen = { egg: { chicken: hen }},

it was creating

henCopy = { egg: { chicken: { egg: { chicken: { egg: { chicken: { egg: ... }}}}}}} .

Since that change, and continuing with the same example, it processes hen and hen.egg, but when it goes to process hen.egg.chicken, it recognizes it as the same hen it's already seen and instead of recursing into it just returns the existing one to complete the cycle. Unfortunately, that means it ends up creating

henCopy = { egg: { chicken: hen }}

rather than

henCopy = { egg: { chicken: henCopy }} ,

thereby violating the "all members of the new cycle must be part of the new object" rule.

This fixes that by using a map to store the new version of each value alongside its original. Now when a cycle is detected, instead of the original being returned, the copy is returned, completing the cycle entirely within the new object.

---

PR description courtesy of @lobsterkatie

Ref: https://getsentry.atlassian.net/browse/WEB-940

This PR should be followed up by: #5171

[github-actions]

size-limit report 📦

Path	Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified)	19.38 KB (-3.76% 🔽)
@sentry/browser - ES5 CDN Bundle (minified)	60.02 KB (-7.1% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)	18.19 KB (-3.57% 🔽)
@sentry/browser - ES6 CDN Bundle (minified)	53.67 KB (-7.43% 🔽)
@sentry/browser - Webpack (gzipped + minified)	19.95 KB (-14.15% 🔽)
@sentry/browser - Webpack (minified)	65.02 KB (-20.44% 🔽)
@sentry/react - Webpack (gzipped + minified)	19.97 KB (-14.19% 🔽)
@sentry/nextjs Client - Webpack (gzipped + minified)	43.72 KB (-9.03% 🔽)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)	25.41 KB (-2.57% 🔽)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)	23.94 KB (-2.23% 🔽)

[AbhiPrasad]

nice, thanks for the detailed write up!