sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

fix(v8/utils): Stack parser skip frames (not lines of stack string)

Open krystofwoldrich opened this issue 1 year ago • 2 comments

  • This PR fixes https://github.com/getsentry/sentry-javascript/issues/9656

In happy scenarios, the framesToPop should not make a difference as the Sentry backend will mark nonInApp frames and hide them by default from the user.

In other scenarios, when source maps are missing for example, the frames will be visible and shown as the root of an error which is misleading and confusing to our users.

Before this PR

We used framesToPop for two unrelated purposes.

  • To skip lines in the stack string that could potentially be parsed as frames although they are part of the message string.
  • To skip n number of frames.

After this PR

We have two variables each having one purpose.

  • skipFirstLines to skip potentially dangerous lines that could be parsed as frames but aren't frames.
  • framesToPop to remove parsed frames.

Notes

This is especially useful for React applications as the framework internally uses the framesToPop property. And also for our own SDK.

  • https://github.com/zertosh/invariant/blob/master/invariant.js#L46
  • https://github.com/getsentry/sentry-react-native/pull/3423

krystofwoldrich avatar Feb 07 '24 21:02 krystofwoldrich

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 77.4 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 68.6 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing, Replay with Canvas) - Webpack (gzipped) 72.51 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.16 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 32.85 KB (+0.05% 🔺)
@sentry/browser (incl. browserTracingIntegration) - Webpack (gzipped) 32.85 KB (+0.05% 🔺)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 31.36 KB (+0.06% 🔺)
@sentry/browser (incl. sendFeedback) - Webpack (gzipped) 31.37 KB (+0.06% 🔺)
@sentry/browser - Webpack (gzipped) 22.57 KB (+0.06% 🔺)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 75.47 KB (+0.06% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 67.08 KB (+0.04% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 32.94 KB (+0.1% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped) 23.98 KB (+0.04% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 210.59 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 99.39 KB (+0.05% 🔺)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 71.67 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 36.09 KB (+0.11% 🔺)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 68.88 KB (+0.02% 🔺)
@sentry/react - Webpack (gzipped) 22.6 KB (+0.06% 🔺)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 85.3 KB (+0.02% 🔺)
@sentry/nextjs Client - Webpack (gzipped) 49.72 KB (+0.03% 🔺)
@sentry-internal/feedback - Webpack (gzipped) 17.41 KB (0%)

github-actions[bot] avatar Feb 07 '24 21:02 github-actions[bot]

@AbhiPrasad I've added the migration information, let me know if it's understandable, or if I should include an example.

krystofwoldrich avatar Feb 13 '24 12:02 krystofwoldrich