Skip to content

Commit c135729

Browse files
committed
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
1 parent 05ab1da commit c135729

2 files changed

Lines changed: 135 additions & 2 deletions

File tree

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,11 @@ export class WhenAllTask<T> extends CompositeTask<T[]> {
1111
constructor(tasks: Task<T>[]) {
1212
super(tasks);
1313

14-
this._completedTasks = 0;
15-
this._failedTasks = 0;
14+
// Note: Do NOT re-initialize _completedTasks or _failedTasks here.
15+
// CompositeTask's constructor already initializes them to 0 and then
16+
// processes pre-completed children via onChildCompleted(), which
17+
// increments the counter. Re-initializing would wipe out that count
18+
// and cause the task to hang when some children are already complete.
1619

1720
// An empty task list should complete immediately with an empty result
1821
if (tasks.length === 0) {
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { WhenAllTask } from "../src/task/when-all-task";
5+
import { CompletableTask } from "../src/task/completable-task";
6+
import { Task } from "../src/task/task";
7+
8+
describe("WhenAllTask", () => {
9+
it("should complete immediately when given an empty task array", () => {
10+
const task = new WhenAllTask<number>([]);
11+
12+
expect(task.isComplete).toBe(true);
13+
expect(task.isFailed).toBe(false);
14+
expect(task.getResult()).toEqual([]);
15+
});
16+
17+
it("should complete when all pending children complete", () => {
18+
const child1 = new CompletableTask<number>();
19+
const child2 = new CompletableTask<number>();
20+
const task = new WhenAllTask([child1, child2]);
21+
22+
expect(task.isComplete).toBe(false);
23+
24+
child1.complete(1);
25+
expect(task.isComplete).toBe(false);
26+
27+
child2.complete(2);
28+
expect(task.isComplete).toBe(true);
29+
expect(task.isFailed).toBe(false);
30+
expect(task.getResult()).toEqual([1, 2]);
31+
});
32+
33+
it("should fail fast when any child fails", () => {
34+
const child1 = new CompletableTask<number>();
35+
const child2 = new CompletableTask<number>();
36+
const task = new WhenAllTask([child1, child2]);
37+
38+
child1.fail("child failed");
39+
40+
expect(task.isComplete).toBe(true);
41+
expect(task.isFailed).toBe(true);
42+
expect(task.getException()).toBeDefined();
43+
});
44+
45+
// Issue #131: WhenAllTask constructor resets _completedTasks counter
46+
it("should complete correctly when constructed with pre-completed children", () => {
47+
const child1 = new CompletableTask<number>();
48+
const child2 = new CompletableTask<number>();
49+
const child3 = new CompletableTask<number>();
50+
51+
// Complete child1 and child2 before constructing WhenAllTask
52+
child1.complete(10);
53+
child2.complete(20);
54+
55+
const task = new WhenAllTask([child1, child2, child3]);
56+
57+
// 2 of 3 children already complete — task should not be complete yet
58+
expect(task.isComplete).toBe(false);
59+
expect(task.completedTasks).toBe(2);
60+
61+
// Complete the last child
62+
child3.complete(30);
63+
64+
expect(task.isComplete).toBe(true);
65+
expect(task.isFailed).toBe(false);
66+
expect(task.getResult()).toEqual([10, 20, 30]);
67+
});
68+
69+
it("should complete immediately when all children are pre-completed", () => {
70+
const child1 = new CompletableTask<number>();
71+
const child2 = new CompletableTask<number>();
72+
73+
child1.complete(1);
74+
child2.complete(2);
75+
76+
const task = new WhenAllTask([child1, child2]);
77+
78+
expect(task.isComplete).toBe(true);
79+
expect(task.isFailed).toBe(false);
80+
expect(task.completedTasks).toBe(2);
81+
expect(task.getResult()).toEqual([1, 2]);
82+
});
83+
84+
it("should fail immediately when a pre-completed child is failed", () => {
85+
const child1 = new CompletableTask<number>();
86+
const child2 = new CompletableTask<number>();
87+
88+
child1.fail("pre-failed");
89+
90+
const task = new WhenAllTask([child1, child2]);
91+
92+
expect(task.isComplete).toBe(true);
93+
expect(task.isFailed).toBe(true);
94+
expect(task.getException()).toBeDefined();
95+
});
96+
97+
it("should not double-complete when child completes after fail-fast", () => {
98+
const child1 = new CompletableTask<number>();
99+
const child2 = new CompletableTask<number>();
100+
const task = new WhenAllTask([child1, child2]);
101+
102+
child1.fail("first failure");
103+
104+
expect(task.isComplete).toBe(true);
105+
expect(task.isFailed).toBe(true);
106+
107+
// Completing child2 after fail-fast should not change the result
108+
child2.complete(2);
109+
expect(task.isFailed).toBe(true);
110+
expect(task.getException()).toBeDefined();
111+
});
112+
113+
it("should report correct pending tasks count", () => {
114+
const child1 = new CompletableTask<number>();
115+
const child2 = new CompletableTask<number>();
116+
const child3 = new CompletableTask<number>();
117+
118+
child1.complete(1);
119+
120+
const task = new WhenAllTask([child1, child2, child3]);
121+
122+
expect(task.pendingTasks()).toBe(2);
123+
124+
child2.complete(2);
125+
expect(task.pendingTasks()).toBe(1);
126+
127+
child3.complete(3);
128+
expect(task.pendingTasks()).toBe(0);
129+
});
130+
});

0 commit comments

Comments
 (0)