Skip to content

Commit 7b74d65

Browse files
committed
[copilot-finds] Bug: Missing non-Task validation in generator failure recovery path
When an orchestrator catches a failed task exception and then yields a non-Task value, the success path in resume() correctly throws an error ('The orchestrator generator yielded a non-Task object'), but the failure recovery path silently returns without updating _previousTask. This leaves the orchestration context in a broken state where: 1. _previousTask still points to the old failed task 2. The next resume() call re-throws the same exception to the generator 3. The orchestration gets stuck or produces confusing errors The fix adds the same non-Task validation to the failure recovery path, making it consistent with the success path. Two new tests verify both the error case and the normal recovery case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 51c41cb commit 7b74d65

2 files changed

Lines changed: 75 additions & 0 deletions

File tree

packages/durabletask-js/src/worker/runtime-orchestration-context.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,8 @@ export class RuntimeOrchestrationContext extends OrchestrationContext {
164164
}
165165
return;
166166
}
167+
168+
throw new Error("The orchestrator generator yielded a non-Task object");
167169
} else if (this._previousTask.isComplete) {
168170
while (true) {
169171
// Resume the generator. This will either return a Task or raise StopIteration if it's done.

packages/durabletask-js/test/orchestration_executor.spec.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,6 +1585,79 @@ describe("Orchestration Executor", () => {
15851585
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_COMPLETED);
15861586
expect(completeAction?.getResult()?.getValue()).toEqual(JSON.stringify("inner_failed"));
15871587
});
1588+
1589+
it("should fail the orchestration when a generator catches a task failure and yields a non-Task value", async () => {
1590+
const dummyActivity = async (_: ActivityContext) => {
1591+
// do nothing
1592+
};
1593+
// This orchestrator catches the activity failure but then yields a non-Task value (a plain number)
1594+
const orchestrator: TOrchestrator = async function* (ctx: OrchestrationContext, _: any): any {
1595+
try {
1596+
yield ctx.callActivity(dummyActivity);
1597+
} catch {
1598+
yield 42 as any; // Bug: yielding a non-Task value after catching an exception
1599+
}
1600+
return "done";
1601+
};
1602+
1603+
const registry = new Registry();
1604+
const name = registry.addOrchestrator(orchestrator);
1605+
const activityName = registry.addActivity(dummyActivity);
1606+
const oldEvents = [
1607+
newOrchestratorStartedEvent(),
1608+
newExecutionStartedEvent(name, TEST_INSTANCE_ID, undefined),
1609+
newTaskScheduledEvent(1, activityName),
1610+
];
1611+
const ex = new Error("Activity failed");
1612+
const newEvents = [newTaskFailedEvent(1, ex)];
1613+
const executor = new OrchestrationExecutor(registry, testLogger);
1614+
const result = await executor.execute(TEST_INSTANCE_ID, oldEvents, newEvents);
1615+
1616+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1617+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1618+
expect(completeAction?.getFailuredetails()?.getErrormessage()).toContain("non-Task");
1619+
});
1620+
1621+
it("should succeed when a generator catches a task failure and yields a valid new Task", async () => {
1622+
const failingActivity = async (_: ActivityContext) => {
1623+
// will fail
1624+
};
1625+
const recoveryActivity = async (_: ActivityContext) => {
1626+
// will succeed
1627+
};
1628+
// This orchestrator catches the activity failure and recovers by calling another activity
1629+
const orchestrator: TOrchestrator = async function* (ctx: OrchestrationContext, _: any): any {
1630+
try {
1631+
yield ctx.callActivity(failingActivity);
1632+
} catch {
1633+
const result = yield ctx.callActivity(recoveryActivity);
1634+
return result;
1635+
}
1636+
};
1637+
1638+
const registry = new Registry();
1639+
const name = registry.addOrchestrator(orchestrator);
1640+
const failingName = registry.addActivity(failingActivity);
1641+
const recoveryName = registry.addActivity(recoveryActivity);
1642+
const oldEvents = [
1643+
newOrchestratorStartedEvent(),
1644+
newExecutionStartedEvent(name, TEST_INSTANCE_ID, undefined),
1645+
newTaskScheduledEvent(1, failingName),
1646+
];
1647+
const ex = new Error("Activity failed");
1648+
const newEvents = [
1649+
newTaskFailedEvent(1, ex),
1650+
// After the failure is caught, the orchestrator schedules recoveryActivity
1651+
newTaskScheduledEvent(2, recoveryName),
1652+
newTaskCompletedEvent(2, JSON.stringify("recovered")),
1653+
];
1654+
const executor = new OrchestrationExecutor(registry, testLogger);
1655+
const result = await executor.execute(TEST_INSTANCE_ID, oldEvents, newEvents);
1656+
1657+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1658+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_COMPLETED);
1659+
expect(completeAction?.getResult()?.getValue()).toEqual(JSON.stringify("recovered"));
1660+
});
15881661
});
15891662

15901663
function getAndValidateSingleCompleteOrchestrationAction(

0 commit comments

Comments
 (0)