From c13572958e0defc44e7cfe75ce3322628ad74403 Mon Sep 17 00:00:00 2001 From: peterstone2017 <12449837+YunchuWang@users.noreply.github.com> Date: Thu, 5 Mar 2026 16:41:02 -0800 Subject: [PATCH 1/3] Fix WhenAllTask constructor resetting _completedTasks counter Fixes #131 The WhenAllTask constructor redundantly re-initialized _completedTasks and _failedTasks to 0 after calling super(tasks). Since CompositeTask's constructor already initializes these fields and then processes pre-completed children via onChildCompleted(), the reset wiped out the correct count, causing WhenAllTask to never complete when some children were already complete at construction time. Also removes _failedTasks reset (dead code - never incremented anywhere). Added 8 unit tests for WhenAllTask covering: - Empty task array - All pending children completing - Fail-fast on child failure - Pre-completed children (the bug scenario) - All children pre-completed - Pre-failed child - Post-fail-fast completion - Pending tasks count tracking --- .../durabletask-js/src/task/when-all-task.ts | 7 +- .../durabletask-js/test/when-all-task.spec.ts | 130 ++++++++++++++++++ 2 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 packages/durabletask-js/test/when-all-task.spec.ts 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..d499ae6 --- /dev/null +++ b/packages/durabletask-js/test/when-all-task.spec.ts @@ -0,0 +1,130 @@ +// 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"; +import { Task } from "../src/task/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); + }); +}); From 88a5e5d272270b5cee47c6ee628e43046b9efe2e Mon Sep 17 00:00:00 2001 From: wangbill Date: Thu, 5 Mar 2026 16:44:09 -0800 Subject: [PATCH 2/3] Potential fix for pull request finding 'Unused variable, import, function or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- packages/durabletask-js/test/when-all-task.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/durabletask-js/test/when-all-task.spec.ts b/packages/durabletask-js/test/when-all-task.spec.ts index d499ae6..a9e4b97 100644 --- a/packages/durabletask-js/test/when-all-task.spec.ts +++ b/packages/durabletask-js/test/when-all-task.spec.ts @@ -3,7 +3,6 @@ import { WhenAllTask } from "../src/task/when-all-task"; import { CompletableTask } from "../src/task/completable-task"; -import { Task } from "../src/task/task"; describe("WhenAllTask", () => { it("should complete immediately when given an empty task array", () => { From 8b2e549c1c7f7dc50033cd468489d1d34540978d Mon Sep 17 00:00:00 2001 From: wangbill Date: Sun, 8 Mar 2026 19:22:33 -0700 Subject: [PATCH 3/3] test: add e2e test for WhenAllTask pre-completed children fix (issue #131) Adds an Azure Managed e2e test that validates whenAll completes correctly when children finish at different speeds and replay occurs. During replay, all whenAll children are already completed, which previously caused the counter reset bug to hang the orchestration. --- test/e2e-azuremanaged/orchestration.spec.ts | 63 +++++++++++++++++++++ 1 file changed, 63 insertions(+) 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;