fix(sdk): keep replay mode active after a virtual context#697
Conversation
checkAndUpdateReplayMode() decided replay was finished whenever the next
step id had no checkpoint data. A virtual context (runInChildContext with
virtualContext: true) is intentionally not checkpointed, so its id has no
data even though its first child IS checkpointed under `${nextStepId}-1`.
This caused replay mode to switch to execution mode immediately after a
virtual context, so operations like logging re-ran (e.g. duplicate logs)
during replay.
Fix: when the next step id has no checkpoint data, also probe its first
child id before concluding that replay has finished. Composes correctly
for root (1 -> 1-1) and nested (2-1 -> 2-1-1) step prefixes.
Adds unit tests covering both the virtual-context case (stays in replay
mode) and the genuine end-of-replay case (switches to execution mode).
Closes #578
SilanHe
left a comment
There was a problem hiding this comment.
Does this change affect logging? I was under impression this is considered backwards incompatible for logging from our previous discussions as it will result in less logs than before. I can accept this as a bugfix.
It affects logging, and you're right that the direction is fewer logs. Before this change, replay mode was incorrectly switching to execution mode right after a virtual context, so This PR restores the intended suppression, so those duplicate replay logs go away. Agreed it's reasonable to take this as a bugfix (it's the fix for #578). |
checkAndUpdateReplayMode() decided replay was finished whenever the next step id had no checkpoint data. A virtual context (runInChildContext with virtualContext: true) is intentionally not checkpointed, so its id has no data even though its first child IS checkpointed under
${nextStepId}-1. This caused replay mode to switch to execution mode immediately after a virtual context, so operations like logging re-ran (e.g. duplicate logs) during replay.Fix: when the next step id has no checkpoint data, also probe its first child id before concluding that replay has finished. Composes correctly for root (1 -> 1-1) and nested (2-1 -> 2-1-1) step prefixes.
Adds unit tests covering both the virtual-context case (stays in replay mode) and the genuine end-of-replay case (switches to execution mode).
Issue #, if available: #578
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.