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

fix(core): Ensure debugIds are applied to all exceptions in an event #14881

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Jan 2, 2025

While investigating #14841, I noticed that we had some brittle non-null assertions in our applyDebugIds function which would cause the debug id application logic to terminate early, in case we'd encounter an event.exception.values item without a stack trace. The added regression test illustrates the scenario in which debug ids would not have been applied to the second exception prior to this fix.

The nice thing about this fix is that it should even further decrease bundle size than the try/catch logic we had before because we can now use optional chaining in v9.

(I'll probably open another PR for the same problem in applyDebugMeta) Added to this PR as it really belonged together to get debugIds working in this scenario.

@Lms24 Lms24 self-assigned this Jan 2, 2025
@Lms24 Lms24 requested review from mydea and lforst January 2, 2025 09:13
Copy link
Contributor

github-actions bot commented Jan 2, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.03 KB +0.83% +194 B 🔺
@sentry/browser - with treeshaking flags 21.75 KB +0.75% +164 B 🔺
@sentry/browser (incl. Tracing) 35.56 KB +0.41% +147 B 🔺
@sentry/browser (incl. Tracing, Replay) 72.87 KB +0.22% +158 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 63.26 KB +0.26% +162 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas) 77.26 KB +0.2% +152 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 89.64 KB +0.17% +153 B 🔺
@sentry/browser (incl. Feedback) 39.78 KB +0.42% +170 B 🔺
@sentry/browser (incl. sendFeedback) 27.65 KB +0.59% +166 B 🔺
@sentry/browser (incl. FeedbackAsync) 32.42 KB +0.52% +170 B 🔺
@sentry/react 25.78 KB +0.63% +164 B 🔺
@sentry/react (incl. Tracing) 38.41 KB +0.44% +169 B 🔺
@sentry/vue 27.27 KB +0.56% +153 B 🔺
@sentry/vue (incl. Tracing) 37.43 KB +0.4% +151 B 🔺
@sentry/svelte 23.2 KB +0.79% +184 B 🔺
CDN Bundle 24.4 KB +0.7% +172 B 🔺
CDN Bundle (incl. Tracing) 35.94 KB +0.49% +178 B 🔺
CDN Bundle (incl. Tracing, Replay) 71.05 KB +0.23% +162 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) 76.27 KB +0.24% +185 B 🔺
CDN Bundle - uncompressed 71.54 KB +0.75% +544 B 🔺
CDN Bundle (incl. Tracing) - uncompressed 107 KB +0.5% +544 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed 221.24 KB +0.25% +544 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 234.11 KB +0.23% +544 B 🔺
@sentry/nextjs (client) 38.7 KB +0.43% +168 B 🔺
@sentry/sveltekit (client) 36.11 KB +0.45% +165 B 🔺
@sentry/node 163.01 KB +0.03% +42 B 🔺
@sentry/node - without tracing 98.76 KB +0.03% +24 B 🔺
@sentry/aws-serverless 128.6 KB +0.02% +20 B 🔺

View base workflow run

@Lms24
Copy link
Member Author

Lms24 commented Jan 2, 2025

Wait, how does this increase bundle size? 😢

Update: Looks like we accidentally still polyfill _optionalChain via sucrase. Which will be adressed via #14882

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