fix(workflow): Fix error when timer is cancelled and immediately fired in the same activation #466
Conversation
…d in the same activation
| makeActivation( | ||
| Date.now(), | ||
| { | ||
| signalWorkflow: { signalName: 'unblock', input: [] }, | ||
| }, | ||
| makeFireTimerJob(1) | ||
| ) | ||
| ); | ||
| compareCompletion(t, completion, makeSuccess([{ cancelTimer: { seq: 1 } }])); |
There was a problem hiding this comment.
How did you make this repro w/o the extra activation?
There was a problem hiding this comment.
"extra"? I think w/ these two jobs:
{
signalWorkflow: { signalName: 'unblock', input: [] },
},
makeFireTimerJob(1)
It processes the first, which results in a cancel (and immediate cleanup/removal of timer completion), and then when it tries to process the second, it drops it because it can't find the completion (and assumes it must have been canceled). (or in the repro case, throws)
There was a problem hiding this comment.
The issue I had initially is that the fireTimer job doesn't get delivered if the workflow is already complete.
Adding another condition at the end of the workflow prevents that.
|
|
||
| /** Consume a completion if it exists in Workflow state, throws if it doesn't */ | ||
| export function consumeCompletion(type: keyof State['completions'], taskSeq: number): Completion { | ||
| const completion = maybeConsumeCompletion(type, taskSeq); | ||
| if (completion === undefined) { | ||
| throw new IllegalStateError(`No completion for taskSeq ${taskSeq}`); |
There was a problem hiding this comment.
I think we should change that message too. "Completion" is a confusing word there. It's more like "No pending awaitable for...", no?
Now, are there any cases in which a user would see this error?
There was a problem hiding this comment.
A user could see this error if new workflow code processes history generated by old code.
No description provided.