Fix WhenAllTask constructor resetting _completedTasks counter

packages/durabletask-js/src/task/when-all-task.ts

@@ -11,8 +11,11 @@ export class WhenAllTask<T> extends CompositeTask<T[]> {
   constructor(tasks: Task<T>[]) {
     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) {

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<number>([]);
+
+    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<number>();
+    const child2 = new CompletableTask<number>();
+    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<number>();
+    const child2 = new CompletableTask<number>();
+    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<number>();
+    const child2 = new CompletableTask<number>();
+    const child3 = new CompletableTask<number>();
+
+    // 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<number>();
+    const child2 = new CompletableTask<number>();
+
+    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<number>();
+    const child2 = new CompletableTask<number>();
+
+    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<number>();
+    const child2 = new CompletableTask<number>();
+    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<number>();
+    const child2 = new CompletableTask<number>();
+    const child3 = new CompletableTask<number>();
+
+    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);
+  });
+});

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<string> => {
+      return "fast";
+    };
+
+    const medium = async (_: ActivityContext): Promise<string> => {
+      await new Promise((resolve) => setTimeout(resolve, 500));
+      return "medium";
+    };
+
+    const slow = async (_: ActivityContext): Promise<string> => {
+      await new Promise((resolve) => setTimeout(resolve, 1500));
+      return "slow";
+    };
+
+    const finalStep = async (_: ActivityContext): Promise<string> => {
+      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;