From f3f170f540129ecd951e6801c2dd27966e0ba941 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sat, 7 Mar 2026 08:27:00 +0000 Subject: [PATCH] fix: sub-orchestration watcher uses unintended 30s default timeout in 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> --- .../src/testing/in-memory-backend.ts | 30 ++++--- .../test/in-memory-backend.spec.ts | 86 +++++++++++++++++++ 2 files changed, 103 insertions(+), 13 deletions(-) diff --git a/packages/durabletask-js/src/testing/in-memory-backend.ts b/packages/durabletask-js/src/testing/in-memory-backend.ts index 4d88923..db068b7 100644 --- a/packages/durabletask-js/src/testing/in-memory-backend.ts +++ b/packages/durabletask-js/src/testing/in-memory-backend.ts @@ -337,24 +337,29 @@ export class InMemoryOrchestrationBackend { } return new Promise((resolve, reject) => { - const timer = setTimeout(() => { - const waiters = this.stateWaiters.get(instanceId); - if (waiters) { - const index = waiters.findIndex((w) => w.resolve === resolve); - if (index >= 0) { - waiters.splice(index, 1); + // When timeoutMs is 0, no timeout is applied — the waiter will only be + // resolved by a matching state change or rejected by reset(). + let timer: ReturnType | undefined; + if (timeoutMs > 0) { + timer = setTimeout(() => { + const waiters = this.stateWaiters.get(instanceId); + if (waiters) { + const index = waiters.findIndex((w) => w.resolve === resolve); + if (index >= 0) { + waiters.splice(index, 1); + } } - } - reject(new Error(`Timeout waiting for orchestration '${instanceId}'`)); - }, timeoutMs); + reject(new Error(`Timeout waiting for orchestration '${instanceId}'`)); + }, timeoutMs); + } const waiter: StateWaiter = { resolve: (result) => { - clearTimeout(timer); + if (timer !== undefined) clearTimeout(timer); resolve(result); }, reject: (error) => { - clearTimeout(timer); + if (timer !== undefined) clearTimeout(timer); reject(error); }, predicate, @@ -590,8 +595,7 @@ export class InMemoryOrchestrationBackend { 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 ) .then((subInstance) => { const parentInstance = this.instances.get(parentInstanceId); diff --git a/packages/durabletask-js/test/in-memory-backend.spec.ts b/packages/durabletask-js/test/in-memory-backend.spec.ts index 502e696..6dbac27 100644 --- a/packages/durabletask-js/test/in-memory-backend.spec.ts +++ b/packages/durabletask-js/test/in-memory-backend.spec.ts @@ -146,6 +146,56 @@ describe("In-Memory Backend", () => { expect(activityCounter).toEqual(1); }); + it("should handle sub-orchestrations with timer delays", async () => { + const childWithTimer: TOrchestrator = async function* (ctx: OrchestrationContext): any { + // Sub-orchestration uses a short timer before returning a result + yield ctx.createTimer(0.1); + return "child-done"; + }; + + const parentOrchestrator: TOrchestrator = async function* (ctx: OrchestrationContext): any { + const result = yield ctx.callSubOrchestrator(childWithTimer); + return `parent-received-${result}`; + }; + + worker.addOrchestrator(childWithTimer); + worker.addOrchestrator(parentOrchestrator); + await worker.start(); + + const id = await client.scheduleNewOrchestration(parentOrchestrator); + const state = await client.waitForOrchestrationCompletion(id, true, 10); + + expect(state).toBeDefined(); + expect(state?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED); + expect(state?.serializedOutput).toEqual(JSON.stringify("parent-received-child-done")); + }); + + it("should handle sub-orchestration failure", async () => { + const failingChild: TOrchestrator = async (_ctx: OrchestrationContext) => { + throw new Error("child failed"); + }; + + const parentOrchestrator: TOrchestrator = async function* (ctx: OrchestrationContext): any { + try { + yield ctx.callSubOrchestrator(failingChild); + return "should not reach"; + } catch (error: any) { + return `caught: ${error.message}`; + } + }; + + worker.addOrchestrator(failingChild); + worker.addOrchestrator(parentOrchestrator); + await worker.start(); + + const id = await client.scheduleNewOrchestration(parentOrchestrator); + const state = await client.waitForOrchestrationCompletion(id, true, 10); + + expect(state).toBeDefined(); + expect(state?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED); + expect(state?.serializedOutput).toContain("caught:"); + }); + it("should handle external events", async () => { const orchestrator: TOrchestrator = async function* (ctx: OrchestrationContext): any { const value = yield ctx.waitForExternalEvent("my_event"); @@ -351,4 +401,40 @@ describe("In-Memory Backend", () => { expect(state?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED); expect(state?.serializedOutput).toEqual(JSON.stringify(42)); }); + + it("waitForState with zero timeout should wait indefinitely until state matches", async () => { + const orchestrator: TOrchestrator = async (_: OrchestrationContext) => "done"; + + worker.addOrchestrator(orchestrator); + await worker.start(); + + const id = await client.scheduleNewOrchestration(orchestrator); + + // Use waitForState with timeoutMs=0 (no timeout). + // The orchestration completes quickly, so this should resolve. + const instance = await backend.waitForState( + id, + (inst) => backend.toClientStatus(inst.status) === OrchestrationStatus.COMPLETED, + 0, + ); + + expect(instance).toBeDefined(); + }); + + it("waitForState with zero timeout should be rejected on reset", async () => { + // Create an instance that won't complete (no worker started) + backend.createInstance("stuck-instance", "test", JSON.stringify("input")); + + // Start waiting with no timeout + const waitPromise = backend.waitForState( + "stuck-instance", + () => false, // Never matches + 0, + ); + + // Reset should reject the waiter + backend.reset(); + + await expect(waitPromise).rejects.toThrow("Backend was reset"); + }); });