feat(replay): Add flush method to integration
#6776
Conversation
size-limit report 📦
|
| if (!this._replay || !this._replay.isEnabled()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
l: Just wondering if we should check for isPaused() here as well... technically, recording could be paused b/c of rate limiting and we wouldn't want to flush in this case. But overall, I think chances are pretty low that this would actually happen. And even if it does, the consequences wouldn't be bad, as we're already handling rate limit responses while being rate-limited. WDYT?
There was a problem hiding this comment.
Yeah, I thought the same. I'd say it is OK, as (theoretically) when paused the event buffer has to be empty always, so it wouldn't even attempt to send anything. I'd say it is OK to leave it like this?
There was a problem hiding this comment.
when paused the event buffer has to be empty always
Not necessarily, pause just stops DOM recording, we could have events from window.performance, but in that case, might be ok to try to flush.
There was a problem hiding this comment.
hmm yeah, true, I'd say that is not incorrect to flush the stuff that has not been sent yet due to being paused?
| if (!this._replay || !this._replay.isEnabled()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
when paused the event buffer has to be empty always
Not necessarily, pause just stops DOM recording, we could have events from window.performance, but in that case, might be ok to try to flush.
| return; | ||
| } | ||
|
|
||
| return this._replay.flushImmediate(); |
There was a problem hiding this comment.
Wouldn't it better to only allow users to use debounced flush to reduce risk of being flooded with flushes?
There was a problem hiding this comment.
Hmm, this would kind of defeat the purpose (a bit) of this - as e.g. if a user clicks a button, we trigger debounced flush, the user closes the window --> we may not have sent the stuff yet. IMHO it's OK to put this burden on the SDK users - it's up to them to only use this sparingly?
This exposes a
flushmethod on the integration class, which can be used to flush any pending replay recording data immediately.This has been requested a few times, and while this PR does not really add a first-class solution for this problem, it at least makes it possible to solve it manually. In combination with
replaysOnErrorSampleRate: 1we can ensure that we always capture up to 60s of replay data. Then, clients may triggerreplay.flush()at any time (e.g. when the user clicks a "submit error" button or similar), sending the buffered replay data.note: I went with
flushvs.flushImmediate, as I'd say that is an internal concern of the replay container class - from a users perspective,flushmakes sense here IMHO?ref #6537
ref #6420