diff --git a/packages/durabletask-js/src/task/when-all-task.ts b/packages/durabletask-js/src/task/when-all-task.ts index e0646d5..1b13f2c 100644 --- a/packages/durabletask-js/src/task/when-all-task.ts +++ b/packages/durabletask-js/src/task/when-all-task.ts @@ -11,8 +11,11 @@ export class WhenAllTask extends CompositeTask { constructor(tasks: Task[]) { super(tasks); - this._completedTasks = 0; - this._failedTasks = 0; + // Note: Do NOT re-initialize _completedTasks or _failedTasks here. + // CompositeTask's constructor already initializes them to 0 and then + // processes pre-completed children via onChildCompleted(), which + // increments the counter. Re-initializing would wipe out that count + // and cause the task to hang when some children are already complete. // An empty task list should complete immediately with an empty result if (tasks.length === 0) { diff --git a/packages/durabletask-js/test/when-all-task.spec.ts b/packages/durabletask-js/test/when-all-task.spec.ts new file mode 100644 index 0000000..a9e4b97 --- /dev/null +++ b/packages/durabletask-js/test/when-all-task.spec.ts @@ -0,0 +1,129 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { WhenAllTask } from "../src/task/when-all-task"; +import { CompletableTask } from "../src/task/completable-task"; + +describe("WhenAllTask", () => { + it("should complete immediately when given an empty task array", () => { + const task = new WhenAllTask([]); + + expect(task.isComplete).toBe(true); + expect(task.isFailed).toBe(false); + expect(task.getResult()).toEqual([]); + }); + + it("should complete when all pending children complete", () => { + const child1 = new CompletableTask(); + const child2 = new CompletableTask(); + const task = new WhenAllTask([child1, child2]); + + expect(task.isComplete).toBe(false); + + child1.complete(1); + expect(task.isComplete).toBe(false); + + child2.complete(2); + expect(task.isComplete).toBe(true); + expect(task.isFailed).toBe(false); + expect(task.getResult()).toEqual([1, 2]); + }); + + it("should fail fast when any child fails", () => { + const child1 = new CompletableTask(); + const child2 = new CompletableTask(); + const task = new WhenAllTask([child1, child2]); + + child1.fail("child failed"); + + expect(task.isComplete).toBe(true); + expect(task.isFailed).toBe(true); + expect(task.getException()).toBeDefined(); + }); + + // Issue #131: WhenAllTask constructor resets _completedTasks counter + it("should complete correctly when constructed with pre-completed children", () => { + const child1 = new CompletableTask(); + const child2 = new CompletableTask(); + const child3 = new CompletableTask(); + + // Complete child1 and child2 before constructing WhenAllTask + child1.complete(10); + child2.complete(20); + + const task = new WhenAllTask([child1, child2, child3]); + + // 2 of 3 children already complete — task should not be complete yet + expect(task.isComplete).toBe(false); + expect(task.completedTasks).toBe(2); + + // Complete the last child + child3.complete(30); + + expect(task.isComplete).toBe(true); + expect(task.isFailed).toBe(false); + expect(task.getResult()).toEqual([10, 20, 30]); + }); + + it("should complete immediately when all children are pre-completed", () => { + const child1 = new CompletableTask(); + const child2 = new CompletableTask(); + + child1.complete(1); + child2.complete(2); + + const task = new WhenAllTask([child1, child2]); + + expect(task.isComplete).toBe(true); + expect(task.isFailed).toBe(false); + expect(task.completedTasks).toBe(2); + expect(task.getResult()).toEqual([1, 2]); + }); + + it("should fail immediately when a pre-completed child is failed", () => { + const child1 = new CompletableTask(); + const child2 = new CompletableTask(); + + child1.fail("pre-failed"); + + const task = new WhenAllTask([child1, child2]); + + expect(task.isComplete).toBe(true); + expect(task.isFailed).toBe(true); + expect(task.getException()).toBeDefined(); + }); + + it("should not double-complete when child completes after fail-fast", () => { + const child1 = new CompletableTask(); + const child2 = new CompletableTask(); + const task = new WhenAllTask([child1, child2]); + + child1.fail("first failure"); + + expect(task.isComplete).toBe(true); + expect(task.isFailed).toBe(true); + + // Completing child2 after fail-fast should not change the result + child2.complete(2); + expect(task.isFailed).toBe(true); + expect(task.getException()).toBeDefined(); + }); + + it("should report correct pending tasks count", () => { + const child1 = new CompletableTask(); + const child2 = new CompletableTask(); + const child3 = new CompletableTask(); + + child1.complete(1); + + const task = new WhenAllTask([child1, child2, child3]); + + expect(task.pendingTasks()).toBe(2); + + child2.complete(2); + expect(task.pendingTasks()).toBe(1); + + child3.complete(3); + expect(task.pendingTasks()).toBe(0); + }); +}); diff --git a/test/e2e-azuremanaged/orchestration.spec.ts b/test/e2e-azuremanaged/orchestration.spec.ts index 16f9ee9..690068e 100644 --- a/test/e2e-azuremanaged/orchestration.spec.ts +++ b/test/e2e-azuremanaged/orchestration.spec.ts @@ -302,6 +302,69 @@ describe("Durable Task Scheduler (DTS) E2E Tests", () => { expect(state?.failureDetails?.message).toContain("slow failure as last task"); }, 31000); + // Issue #131: WhenAllTask constructor was resetting the _completedTasks counter, + // causing whenAll to hang when some children were already completed during replay. + // This test validates the fix by scheduling activities that complete at different + // speeds, then doing additional work after whenAll. The additional activity after + // whenAll forces a replay where the whenAll children are already completed, which + // would trigger the bug (the counter reset would make it appear that no children + // had completed, causing whenAll to never resolve). + it("should complete whenAll correctly when children finish at different speeds and replay occurs", async () => { + const fast = async (_: ActivityContext): Promise => { + return "fast"; + }; + + const medium = async (_: ActivityContext): Promise => { + await new Promise((resolve) => setTimeout(resolve, 500)); + return "medium"; + }; + + const slow = async (_: ActivityContext): Promise => { + await new Promise((resolve) => setTimeout(resolve, 1500)); + return "slow"; + }; + + const finalStep = async (_: ActivityContext): Promise => { + return "done"; + }; + + const orchestrator: TOrchestrator = async function* (ctx: OrchestrationContext): any { + // Fan-out with activities completing at different speeds + const results: string[] = yield whenAll([ + ctx.callActivity(fast), + ctx.callActivity(medium), + ctx.callActivity(slow), + ]); + + // This additional activity forces a replay of the orchestration. + // During replay, all three whenAll children will already be completed + // (they have history events). The bug was that WhenAllTask's constructor + // reset the _completedTasks counter AFTER CompositeTask's constructor + // had already counted them, so whenAll would never resolve. + const final: string = yield ctx.callActivity(finalStep); + + return { results, final }; + }; + + taskHubWorker.addActivity(fast); + taskHubWorker.addActivity(medium); + taskHubWorker.addActivity(slow); + taskHubWorker.addActivity(finalStep); + taskHubWorker.addOrchestrator(orchestrator); + await taskHubWorker.start(); + + const id = await taskHubClient.scheduleNewOrchestration(orchestrator); + const state = await taskHubClient.waitForOrchestrationCompletion(id, undefined, 30); + + expect(state).toBeDefined(); + expect(state?.runtimeStatus).toEqual(OrchestrationStatus.ORCHESTRATION_STATUS_COMPLETED); + expect(state?.failureDetails).toBeUndefined(); + + const output = JSON.parse(state?.serializedOutput || "{}"); + expect(output.results).toEqual(["fast", "medium", "slow"]); + expect(output.final).toEqual("done"); + }, 31000); + it("should be able to use the sub-orchestration", async () => { let activityCounter = 0;