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

Improve usability of Replay SDK public APIs (start/stop/flush) #12810

Closed
billyvg opened this issue Jul 8, 2024 · 1 comment · Fixed by #13000
Closed

Improve usability of Replay SDK public APIs (start/stop/flush) #12810

billyvg opened this issue Jul 8, 2024 · 1 comment · Fixed by #13000
Assignees
Labels
Package: replay Issues related to the Sentry Replay SDK

Comments

@billyvg
Copy link
Member

billyvg commented Jul 8, 2024

Below is a state table of our current public APIs. Notably, start() and startBuffering() will throw if replay is already started and flush() is a no-op if replay is currently inactive.

We should change to not throw, and instead use a debug level log if replay has already been started. We want to warn users when they do this, but not throw errors since nothing will break as it can be a no-op when this happens.

Additionally, flush() should start a session replay if it is called when replay is not enabled because if they call flush() we can assume they want a replay.

Current behavior

Stopped Started (session) Started (buffering)
start() Start session recording throws ("already in progress") throws ("buffering in progress")
startBuffering() Start buffering throws ("already in progress") throws ("already in progress")
flush() noop Flushes the current segment immediately Flushes buffered replay immediately + changes to "session" type
stop() noop Stops recording + forces a flush + clears session Stops buffering + clears session

Proposed behavior

Stopped Started (session) Started (buffering)
start() Start session recording debug log ("already in progress") debug log ("buffering in progress")
startBuffering() Start buffering debug log ("already in progress") debug log ("already in progress")
flush() Start session recording Flushes the current segment immediately Flushes buffered replay immediately + changes to "session" type
stop() noop Stops recording + forces a flush + clears session Stops buffering + clears session
@billyvg billyvg added the Package: replay Issues related to the Sentry Replay SDK label Jul 8, 2024
@mydea
Copy link
Member

mydea commented Jul 9, 2024

all of this sounds reasonable to me!
Technically, it is a breaking change (there may be users depending on start throwing, for example) but I guess it's OK anyhow.

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants