fix: Sub-orchestration watcher uses unintended 30s default timeout in InMemoryOrchestrationBackend#156
Merged
YunchuWang merged 2 commits intomainfrom Apr 13, 2026
Conversation
… InMemoryOrchestrationBackend The watchSubOrchestration method in InMemoryOrchestrationBackend intended to wait indefinitely for sub-orchestrations to complete (as documented by the inline comment 'No timeout'), but actually used the default 30-second timeout from waitForState. After 30 seconds, the watcher silently dropped the completion event via .catch(), causing the parent orchestration to hang indefinitely. Fix: - Modified waitForState to support no-timeout mode when timeoutMs is 0 - Updated watchSubOrchestration to pass timeoutMs=0 matching the documented intent - Added tests for sub-orchestrations with timer delays and failure propagation - Added tests for waitForState zero-timeout behavior and cleanup on reset Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an in-memory testing backend bug where sub-orchestration completion watching unintentionally timed out after 30 seconds (and silently swallowed the timeout), preventing the parent orchestration from receiving the child completion/failure event.
Changes:
- Add “no-timeout” support to
InMemoryOrchestrationBackend.waitForStatewhentimeoutMs === 0(skipsetTimeoutcreation). - Update sub-orchestration watcher to pass
0towaitForStateto truly wait indefinitely. - Add unit tests covering sub-orchestration timer delays/failure propagation and
waitForState(..., 0)behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/durabletask-js/src/testing/in-memory-backend.ts | Implements no-timeout mode for state waiting and applies it to sub-orchestration watching. |
| packages/durabletask-js/test/in-memory-backend.spec.ts | Adds regression/behavior tests around sub-orchestrations and zero-timeout waiting. |
Comment on lines
595
to
599
| this.waitForState( | ||
| subInstanceId, | ||
| (inst) => this.isTerminalStatus(inst.status), | ||
| // No timeout - sub-orchestration will eventually complete, fail, or be terminated | ||
| // If parent is terminated, we check that when delivering the event | ||
| 0, // No timeout — sub-orchestration will eventually complete, fail, or be terminated | ||
| ) |
There was a problem hiding this comment.
Now that waitForState(..., 0) can’t time out, the .catch comment below that mentions “Timeout” is misleading. Consider updating the comment to reflect the only expected cancellation path here (backend reset), or explicitly note any other errors you intend to swallow.
kaibocai
approved these changes
Apr 13, 2026
YunchuWang
deleted the
copilot-finds/bug/fix-sub-orchestration-watcher-timeout
branch
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
Fixes #149
Bug:
watchSubOrchestrationinInMemoryOrchestrationBackendintends to wait indefinitely for a sub-orchestration to complete (as documented by the inline comment "No timeout"), but notimeoutMsargument is passed towaitForState, so the default 30-second timeout applies. After 30 seconds, the timeout error is silently swallowed by the.catch(() => {})handler, and the completion/failure event is never delivered to the parent orchestration.Changes
packages/durabletask-js/src/testing/in-memory-backend.ts— ModifiedwaitForStateto support no-timeout mode whentimeoutMsis0(skips creating thesetTimeouttimer); updatedwatchSubOrchestrationto pass0to match the documented intentpackages/durabletask-js/test/in-memory-backend.spec.ts— Added tests verifying sub-orchestrations with timer delays and failure propagation work correctly without the 30s timeoutTesting
All tests pass. Lint clean.