feat: Do not flush immediately on checkout#109
Conversation
Change the logic for when to flush after a full snapshot checkout. Add option `initialFlushDelay` to control when to flush. Do NOT flush on checkout after a session has expired. It will need to wait for additional user actions before recording the new sessions replay.
| flushMinDelay = 5000, | ||
| flushMaxDelay = 15000, |
There was a problem hiding this comment.
Went ahead and renamed this as well.
| } | ||
|
|
||
| if (session.id !== this.session?.id) { | ||
| session.previousSessionId = this.session?.id; |
There was a problem hiding this comment.
Wonder if it'd be useful to have this in our replay events to allow users to paginate through sessions or something cc @JoshFerge
| logger.error( | ||
| new Error('Attempting to finish replay event after session expired.') | ||
| ); | ||
| return; |
There was a problem hiding this comment.
Added this as we should not continue to flush if session is expired.
| { | ||
| data: { isCheckout }, | ||
| timestamp: BASE_TIMESTAMP, | ||
| timestamp: new Date().getTime(), |
There was a problem hiding this comment.
This is more accurate, as we want the current mocked time.
src/index.ts
Outdated
| const result = cb?.(); | ||
|
|
||
| if (typeof result === 'function') { | ||
| result(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
It seems weird to optionally return a function and then stop processing after calling the function.
Would it work if the cb just exec whatever extra statements (the timeout), without the extra function, and return something to control whether we stop processing or not?
The return value could be a Symbol or something to tell it to stop processing, that'd save from having to update all callsites, and would be readable.
There was a problem hiding this comment.
🤨 you're right, I think this was my brain flip flopping around about how I wanted this to work - thanks!
src/index.test.ts
Outdated
| return Promise.resolve(); | ||
| }); | ||
| advanceTimers(5000); | ||
| await advanceTimers(5000); |
There was a problem hiding this comment.
oops. will watch more closely next time
i wonder if sentry repo catches these... it should be able to
There was a problem hiding this comment.
Yeah there's a lint rule for this
Change the logic for when to flush after a full snapshot checkout. Add option `initialFlushDelay` to control when to flush. Do NOT flush on checkout after a session has expired. It will need to wait for additional user actions before recording the new sessions replay.
Change the logic for when to flush after a full snapshot checkout. Add
option
initialFlushDelayto control when to flush.Do NOT flush on checkout after a session has expired. It will need to
wait for additional user actions before recording the new sessions
replay.