Skip to content

Commit 15bb7de

Browse files
committed
fix: validate first yielded value in orchestrator run() method
The run() method in RuntimeOrchestrationContext did not validate that the first value yielded by an orchestrator generator is a Task instance. The resume() method already validates subsequent yields with an instanceof check, but run() silently accepted non-Task values (null, undefined, plain objects, primitives), causing the orchestration to hang indefinitely with no error message. This adds the same validation to run() that already exists in resume(), throwing a clear error when a non-Task value is yielded. This also removes the now-addressed TODO comment and the redundant instanceof guard on the isComplete check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 51c41cb commit 15bb7de

2 files changed

Lines changed: 82 additions & 4 deletions

File tree

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,7 @@ export class RuntimeOrchestrationContext extends OrchestrationContext {
109109
async run(generator: Generator<Task<any>, any, any>) {
110110
this._generator = generator;
111111

112-
// TODO: do something with this task
113-
// start the generator
112+
// Start the generator
114113
const { value, done } = await this._generator.next();
115114

116115
// if the generator finished, complete the orchestration.
@@ -119,12 +118,15 @@ export class RuntimeOrchestrationContext extends OrchestrationContext {
119118
return;
120119
}
121120

122-
// TODO: check if the task is null?
121+
if (!(value instanceof Task)) {
122+
throw new Error("The orchestrator generator yielded a non-Task object");
123+
}
124+
123125
this._previousTask = value;
124126

125127
// If the yielded task is already complete (e.g., whenAll with an empty array),
126128
// resume immediately so the generator can continue.
127-
if (this._previousTask instanceof Task && this._previousTask.isComplete) {
129+
if (this._previousTask.isComplete) {
128130
await this.resume();
129131
}
130132
}

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1549,6 +1549,82 @@ describe("Orchestration Executor", () => {
15491549
expect(completeAction?.getResult()?.getValue()).toEqual(JSON.stringify(expectedResult));
15501550
});
15511551

1552+
it("should fail when orchestrator yields null as its first value", async () => {
1553+
// An orchestrator that yields null instead of a Task should fail with a clear error
1554+
const badOrchestrator: TOrchestrator = async function* (_ctx: OrchestrationContext): any {
1555+
yield null;
1556+
};
1557+
1558+
const registry = new Registry();
1559+
const name = registry.addOrchestrator(badOrchestrator);
1560+
const newEvents = [
1561+
newOrchestratorStartedEvent(),
1562+
newExecutionStartedEvent(name, TEST_INSTANCE_ID, undefined),
1563+
];
1564+
const executor = new OrchestrationExecutor(registry, testLogger);
1565+
const result = await executor.execute(TEST_INSTANCE_ID, [], newEvents);
1566+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1567+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1568+
expect(completeAction?.getFailuredetails()?.getErrormessage()).toContain("non-Task");
1569+
});
1570+
1571+
it("should fail when orchestrator yields undefined as its first value", async () => {
1572+
// An orchestrator that yields undefined instead of a Task should fail with a clear error
1573+
const badOrchestrator: TOrchestrator = async function* (_ctx: OrchestrationContext): any {
1574+
yield undefined;
1575+
};
1576+
1577+
const registry = new Registry();
1578+
const name = registry.addOrchestrator(badOrchestrator);
1579+
const newEvents = [
1580+
newOrchestratorStartedEvent(),
1581+
newExecutionStartedEvent(name, TEST_INSTANCE_ID, undefined),
1582+
];
1583+
const executor = new OrchestrationExecutor(registry, testLogger);
1584+
const result = await executor.execute(TEST_INSTANCE_ID, [], newEvents);
1585+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1586+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1587+
expect(completeAction?.getFailuredetails()?.getErrormessage()).toContain("non-Task");
1588+
});
1589+
1590+
it("should fail when orchestrator yields a plain object as its first value", async () => {
1591+
// An orchestrator that yields a non-Task object should fail with a clear error
1592+
const badOrchestrator: TOrchestrator = async function* (_ctx: OrchestrationContext): any {
1593+
yield { someProperty: "not a task" };
1594+
};
1595+
1596+
const registry = new Registry();
1597+
const name = registry.addOrchestrator(badOrchestrator);
1598+
const newEvents = [
1599+
newOrchestratorStartedEvent(),
1600+
newExecutionStartedEvent(name, TEST_INSTANCE_ID, undefined),
1601+
];
1602+
const executor = new OrchestrationExecutor(registry, testLogger);
1603+
const result = await executor.execute(TEST_INSTANCE_ID, [], newEvents);
1604+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1605+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1606+
expect(completeAction?.getFailuredetails()?.getErrormessage()).toContain("non-Task");
1607+
});
1608+
1609+
it("should fail when orchestrator yields a primitive as its first value", async () => {
1610+
// An orchestrator that yields a primitive (number) instead of a Task should fail
1611+
const badOrchestrator: TOrchestrator = async function* (_ctx: OrchestrationContext): any {
1612+
yield 42;
1613+
};
1614+
1615+
const registry = new Registry();
1616+
const name = registry.addOrchestrator(badOrchestrator);
1617+
const newEvents = [
1618+
newOrchestratorStartedEvent(),
1619+
newExecutionStartedEvent(name, TEST_INSTANCE_ID, undefined),
1620+
];
1621+
const executor = new OrchestrationExecutor(registry, testLogger);
1622+
const result = await executor.execute(TEST_INSTANCE_ID, [], newEvents);
1623+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1624+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1625+
expect(completeAction?.getFailuredetails()?.getErrormessage()).toContain("non-Task");
1626+
});
1627+
15521628
it("should propagate inner whenAll failure to outer whenAny in nested composites", async () => {
15531629
const hello = (_: any, name: string) => {
15541630
return `Hello ${name}!`;

0 commit comments

Comments
 (0)