fix: propagate exceptions from immediately-failed tasks in resume() loop - #329
Open
YunchuWang wants to merge 4 commits into
Open
fix: propagate exceptions from immediately-failed tasks in resume() loop#329YunchuWang wants to merge 4 commits into
YunchuWang wants to merge 4 commits into
Conversation
…y-failed tasks When the resume() method's isComplete while-loop encountered a task that was already failed (e.g., a WhenAllTask with a pre-failed child), it continued looping and called generator.next(undefined) instead of throwing the exception into the generator via generator.throw(). This caused failed task exceptions to be silently lost. The fix adds a check for isFailed inside the while-loop. When an immediately-failed task is detected, it breaks out and delegates to the isFailed handling branch via a recursive resume() call. 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 correctness bug in the orchestration replay/resume loop: when an orchestrator yields a task that is already complete but failed (e.g., whenAll([...]) with a pre-failed child), the runtime now correctly throws the task exception into the generator instead of resuming with an undefined result and silently losing the error. This aligns with the SDK’s generator-based replay model, where failed yielded tasks must surface as exceptions to deterministic orchestrator code.
Changes:
- Update
RuntimeOrchestrationContext.resume()to detect immediately-failed tasks inside the “immediately-complete” loop and route them through the existing failure/generator.throw()path. - Add regression coverage for the pre-failed-child
whenAllscenario, validating both caught-exception and uncaught-exception outcomes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/durabletask-js/src/worker/runtime-orchestration-context.ts | Ensures immediately-failed yielded tasks propagate via generator.throw() instead of being treated like successful completions in the resume loop. |
| packages/durabletask-js/test/orchestration_executor.spec.ts | Adds regression tests that reproduce the original “swallowed exception” scenario and assert correct completion vs failure behavior. |
…task-exception Conflict in orchestration_executor.spec.ts: main added several suites in the same region. Re-applied this branch's two whenAll pre-failed-child tests at the end of the 'Orchestration Executor' suite. Updated the uncaught-failure test to expect AggregateError instead of TaskFailedError, since main (#301) now aggregates whenAll child failures; the aggregate message still inlines 'Activity exploded'. Verified the src fix is still load-bearing: both tests fail without it.
The resume() fast-path loop only checked isComplete, never isFailed, so a task that was already complete AND failed when yielded fell through to generator.next(task._result) with an undefined result. The exception was never thrown into the generator. Existing coverage only exercised the whenAll variant at the executor level. These tests cover the broader and more common trigger end-to-end against the DTS emulator: the ordinary "start early, join later" pattern using a plain callActivity handle, with no whenAll involved. Without the fix the caught variant returns "NO-THROW:undefined" and the uncaught variant reports COMPLETED instead of FAILED. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5aaa70df-fae3-4570-aabe-0fc96cb869bc
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.
Fixes #206.
The
resume()while-loop silently swallowed exceptions thrown by immediately-failed tasks. This propagates those exceptions instead of dropping them.Opens the existing fix branch
copilot-finds/bug/resume-loop-swallows-failed-task-exceptionas a PR (automated triage of active [copilot-finds] issues that had a ready fix branch but no open PR).