Skip to content

Commit 459d64e

Browse files
authored
refactor: remove dead _failedTasks field and unused async-pageable.ts (#284)
1 parent 42524ce commit 459d64e

5 files changed

Lines changed: 45 additions & 95 deletions

File tree

packages/durabletask-js/src/task/composite-task.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,12 @@ import { Task } from "./task";
99
export class CompositeTask<T> extends Task<T> {
1010
_tasks: Task<any>[] = [];
1111
_completedTasks: number;
12-
_failedTasks: number;
1312

1413
constructor(tasks: Task<any>[]) {
1514
super();
1615

1716
this._tasks = tasks;
1817
this._completedTasks = 0;
19-
this._failedTasks = 0;
2018

2119
for (const task of tasks) {
2220
task._parent = this;

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

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

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

packages/durabletask-js/src/utils/async-pageable.ts

Lines changed: 0 additions & 91 deletions
This file was deleted.

packages/durabletask-js/test/when-all-task.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,4 +256,26 @@ describe("WhenAllTask", () => {
256256
expect(err.errors[0]).toBe(error0);
257257
expect(err.errors[1]).toBe(error2);
258258
});
259+
260+
// Issue #217: CompositeTask has no separate failed-task counter. `_completedTasks`
261+
// counts every settled child alike, successful and failed, which is why the unused
262+
// `_failedTasks` field was safe to delete.
263+
it("should count both successful and failed children in completedTasks", () => {
264+
const child1 = new CompletableTask<number>();
265+
const child2 = new CompletableTask<number>();
266+
const task = new WhenAllTask([child1, child2]);
267+
268+
child1.complete(1);
269+
expect(task.completedTasks).toBe(1);
270+
expect(task.pendingTasks()).toBe(1);
271+
272+
// The failing child increments the SAME counter as the successful one.
273+
child2.fail("child failed");
274+
expect(task.completedTasks).toBe(2);
275+
expect(task.pendingTasks()).toBe(0);
276+
277+
// Every child is now settled, so wait-all completes the task as failed.
278+
expect(task.isComplete).toBe(true);
279+
expect(task.isFailed).toBe(true);
280+
});
259281
});

packages/durabletask-js/test/when-any-task.spec.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,4 +148,25 @@ describe("WhenAnyTask", () => {
148148
expect(task.isComplete).toBe(false);
149149
expect(task.isFailed).toBe(false);
150150
});
151+
152+
// Issue #217: a failed completion settles WhenAny immediately and is treated the
153+
// same as a successful one — failures are not tracked separately, so a later
154+
// successful child does not override the already-settled failed child.
155+
it("should keep the first failed child as the result even if a later child succeeds", () => {
156+
const child1 = new CompletableTask<number>();
157+
const child2 = new CompletableTask<number>();
158+
const task = new WhenAnyTask([child1, child2]);
159+
160+
child1.fail("first failed");
161+
162+
expect(task.isComplete).toBe(true);
163+
// WhenAny itself is not failed — it simply returns the settled (failed) child.
164+
expect(task.isFailed).toBe(false);
165+
expect(task.getResult()).toBe(child1);
166+
167+
// A subsequent successful completion must not replace the already-settled failed child.
168+
child2.complete(99);
169+
expect(task.getResult()).toBe(child1);
170+
expect(task.getResult().isFailed).toBe(true);
171+
});
151172
});

0 commit comments

Comments
 (0)