fix: Add missing non-Task validation in generator failure recovery path#163
Merged
YunchuWang merged 3 commits intoMar 11, 2026
Conversation
… recovery path
When an orchestrator catches a failed task exception and then yields a
non-Task value, the success path in resume() correctly throws an error
('The orchestrator generator yielded a non-Task object'), but the failure
recovery path silently returns without updating _previousTask. This
leaves the orchestration context in a broken state where:
1. _previousTask still points to the old failed task
2. The next resume() call re-throws the same exception to the generator
3. The orchestration gets stuck or produces confusing errors
The fix adds the same non-Task validation to the failure recovery path,
making it consistent with the success path. Two new tests verify both
the error case and the normal recovery case.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the orchestration replay engine by making the generator “failure recovery” path validate yielded values the same way the “success” path already does, ensuring that yielding a non-Task after catching a task failure deterministically fails the orchestration (instead of silently returning in a broken state).
Changes:
- Add missing non-
Taskvalidation inRuntimeOrchestrationContext.resume()when resuming viagenerator.throw(...). - Add Jest coverage for (a) yielding a non-
Taskafter catching a failure and (b) yielding a valid recoveryTaskafter catching a failure.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/durabletask-js/src/worker/runtime-orchestration-context.ts | Adds the missing throw for non-Task yields in the failure recovery path of resume(). |
| packages/durabletask-js/test/orchestration_executor.spec.ts | Adds regression tests covering the failure-recovery yield validation behavior. |
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
kaibocai
approved these changes
Mar 11, 2026
This was referenced Apr 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add non-Task validation in the failure recovery path of
esume() to match the existing validation in the success path.
Closes #159
Problem
When an orchestrator catches a failed task exception and yields a non-Task value, the failure recovery path silently returns without error, leaving _previousTask pointing to the old failed task. The success path already correctly validates with hrow new Error('The orchestrator generator yielded a non-Task object').
Fix
Add the same validation error after the instanceof Task check in the failure recovery path.