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(replay): Ensure buffer sessions end after capturing an error #8713

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

mydea
Copy link
Member

@mydea mydea commented Aug 2, 2023

We haven't been persisting the shouldRefresh property of the session. This combined with the fact that we do not update the sampled field on the session to session when an error occurs, but keep it at buffer, means that if a user reloads the page or the session is otherwise re-fetched from sessionStorage, if previously an error occurs, we'll keep buffering forever again (like for a "fresh" buffer session), and if an error happens we convert it again to a session session, but since the session ID was never updated this will be "added" to the previous session instead.

I made a reproduction test that failed before and works after this fix.

Fixes #8400

@mydea mydea added Type: Bug Package: replay Issues related to the Sentry Replay SDK labels Aug 2, 2023
@mydea mydea requested a review from billyvg August 2, 2023 10:50
@mydea mydea self-assigned this Aug 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 75.11 KB (+0.02% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 31.41 KB (0%)
@sentry/browser - Webpack (gzipped) 21.95 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 69.85 KB (+0.02% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 28.29 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped) 20.33 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 221.08 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 86 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 60.48 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 30.41 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 65.17 KB (+0.03% 🔺)
@sentry/react - Webpack (gzipped) 21.96 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 92.94 KB (+0.02% 🔺)
@sentry/nextjs Client - Webpack (gzipped) 50.89 KB (0%)

Copy link
Member

@billyvg billyvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice catch!

@mydea mydea merged commit 2748691 into develop Aug 3, 2023
72 checks passed
@mydea mydea deleted the fn/replay-change-session-type-buffer branch August 3, 2023 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: replay Issues related to the Sentry Replay SDK Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expired sessions are not being recreated correctly leading to very long session durations
2 participants