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

feat(core)!: Always use session from isolation scope #14860

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

Conversation

mydea
Copy link
Member

@mydea mydea commented Dec 30, 2024

This PR ensures that we always take the session from the isolation scope, never from the current scope.

This has the implication that we need to be sure to pass the isolation scope to _processEvent, as this is where the session may be marked as errored. For this, I updated the internal method _processEvent to take the isolation scope as last argument, as well as streamlining this slightly.

I opted to update the signature of the protected _prepareEvent method too, and make currentScope/isolationScope required there. We already always pass this in now, so it safes a few bytes to avoid the fallback everywhere. This should not really affect users unless they overwrite the _processEvent method, which is internal/private anyhow, so IMHO this should be fine. I added a small note to the migration guide anyhow!

@mydea mydea requested review from lforst and Lms24 December 30, 2024 13:58
@mydea mydea self-assigned this Dec 30, 2024
Copy link
Contributor

github-actions bot commented Dec 30, 2024

size-limit report 📦

Path Size % Change Change
@sentry/browser 22.83 KB -0.06% -14 B 🔽
@sentry/browser - with treeshaking flags 21.58 KB -0.05% -10 B 🔽
@sentry/browser (incl. Tracing) 35.41 KB -0.04% -12 B 🔽
@sentry/browser (incl. Tracing, Replay) 72.7 KB -0.02% -12 B 🔽
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 63.09 KB -0.02% -11 B 🔽
@sentry/browser (incl. Tracing, Replay with Canvas) 77.1 KB -0.02% -11 B 🔽
@sentry/browser (incl. Tracing, Replay, Feedback) 89.47 KB -0.02% -13 B 🔽
@sentry/browser (incl. Feedback) 39.59 KB -0.05% -17 B 🔽
@sentry/browser (incl. sendFeedback) 27.48 KB -0.05% -14 B 🔽
@sentry/browser (incl. FeedbackAsync) 32.23 KB -0.05% -16 B 🔽
@sentry/react 25.61 KB -0.02% -5 B 🔽
@sentry/react (incl. Tracing) 38.24 KB -0.02% -5 B 🔽
@sentry/vue 27.12 KB +0.01% +2 B 🔺
@sentry/vue (incl. Tracing) 37.28 KB -0.01% -3 B 🔽
@sentry/svelte 23.01 KB -0.04% -9 B 🔽
CDN Bundle 24.19 KB -0.18% -43 B 🔽
CDN Bundle (incl. Tracing) 35.73 KB -0.12% -43 B 🔽
CDN Bundle (incl. Tracing, Replay) 70.84 KB -0.07% -49 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) 76.04 KB -0.07% -47 B 🔽
CDN Bundle - uncompressed 70.85 KB -0.22% -156 B 🔽
CDN Bundle (incl. Tracing) - uncompressed 106.32 KB -0.15% -156 B 🔽
CDN Bundle (incl. Tracing, Replay) - uncompressed 220.56 KB -0.07% -156 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 233.42 KB -0.07% -156 B 🔽
@sentry/nextjs (client) 38.53 KB -0.02% -5 B 🔽
@sentry/sveltekit (client) 35.94 KB -0.03% -9 B 🔽
@sentry/node 162.95 KB -0.01% -15 B 🔽
@sentry/node - without tracing 98.73 KB -0.02% -16 B 🔽
@sentry/aws-serverless 128.57 KB -0.01% -11 B 🔽

View base workflow run

Copy link

codecov bot commented Dec 30, 2024

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
693 3 690 294
View the top 3 failed tests by shortest run time
sessions/update-session/test.ts should update session when an error is thrown.
Stack Traces | 0.269s run time
test.ts:7:11 should update session when an error is thrown.
sessions/update-session/test.ts should update session when an exception is captured.
Stack Traces | 0.287s run time
test.ts:24:11 should update session when an exception is captured.
client/sessions.test.ts should report crashed sessions
Stack Traces | 30s run time
sessions.test.ts:16:5 should report crashed sessions

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@mydea mydea force-pushed the fn/session-currentScope branch from 0a3e7a2 to c0ca43d Compare December 30, 2024 15:41
mydea added 4 commits January 2, 2025 08:45
This is more behavioural breaking, for general usage this should not change anything.
@mydea mydea force-pushed the fn/session-currentScope branch from d0d8339 to a80d35a Compare January 2, 2025 08:05
@mydea mydea changed the title ref(core): Stop setting session on current scope feat(core): Always use session from isolation scope Jan 2, 2025
@mydea mydea changed the title feat(core): Always use session from isolation scope feat(core)!: Always use session from isolation scope Jan 2, 2025
@mydea mydea requested a review from AbhiPrasad January 2, 2025 08:12
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