Skip to content

Commit 5cc8dd0

Browse files
authored
fix: Validate first yielded value in orchestrator run() method (#164)
1 parent e8ad92f commit 5cc8dd0

2 files changed

Lines changed: 69 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: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1803,6 +1803,69 @@ describe("Orchestration Executor", () => {
18031803
expect(completeAction?.getResult()?.getValue()).toEqual(JSON.stringify(expectedResult));
18041804
});
18051805

1806+
it.each([
1807+
{ description: "null", yieldedValue: null as any },
1808+
{ description: "undefined", yieldedValue: undefined as any },
1809+
])(
1810+
"should fail when orchestrator yields $description as its first value",
1811+
async ({ description: _description, yieldedValue }) => {
1812+
// An orchestrator that yields a non-Task value as its first yield should fail with a clear error
1813+
const badOrchestrator: TOrchestrator = async function* (_ctx: OrchestrationContext): any {
1814+
yield yieldedValue;
1815+
};
1816+
1817+
const registry = new Registry();
1818+
const name = registry.addOrchestrator(badOrchestrator);
1819+
const newEvents = [
1820+
newOrchestratorStartedEvent(),
1821+
newExecutionStartedEvent(name, TEST_INSTANCE_ID, undefined),
1822+
];
1823+
const executor = new OrchestrationExecutor(registry, testLogger);
1824+
const result = await executor.execute(TEST_INSTANCE_ID, [], newEvents);
1825+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1826+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1827+
expect(completeAction?.getFailuredetails()?.getErrormessage()).toContain("non-Task");
1828+
}
1829+
);
1830+
1831+
it("should fail when orchestrator yields a plain object as its first value", async () => {
1832+
// An orchestrator that yields a non-Task object should fail with a clear error
1833+
const badOrchestrator: TOrchestrator = async function* (_ctx: OrchestrationContext): any {
1834+
yield { someProperty: "not a task" };
1835+
};
1836+
1837+
const registry = new Registry();
1838+
const name = registry.addOrchestrator(badOrchestrator);
1839+
const newEvents = [
1840+
newOrchestratorStartedEvent(),
1841+
newExecutionStartedEvent(name, TEST_INSTANCE_ID, undefined),
1842+
];
1843+
const executor = new OrchestrationExecutor(registry, testLogger);
1844+
const result = await executor.execute(TEST_INSTANCE_ID, [], newEvents);
1845+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1846+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1847+
expect(completeAction?.getFailuredetails()?.getErrormessage()).toContain("non-Task");
1848+
});
1849+
1850+
it("should fail when orchestrator yields a primitive as its first value", async () => {
1851+
// An orchestrator that yields a primitive (number) instead of a Task should fail
1852+
const badOrchestrator: TOrchestrator = async function* (_ctx: OrchestrationContext): any {
1853+
yield 42;
1854+
};
1855+
1856+
const registry = new Registry();
1857+
const name = registry.addOrchestrator(badOrchestrator);
1858+
const newEvents = [
1859+
newOrchestratorStartedEvent(),
1860+
newExecutionStartedEvent(name, TEST_INSTANCE_ID, undefined),
1861+
];
1862+
const executor = new OrchestrationExecutor(registry, testLogger);
1863+
const result = await executor.execute(TEST_INSTANCE_ID, [], newEvents);
1864+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1865+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1866+
expect(completeAction?.getFailuredetails()?.getErrormessage()).toContain("non-Task");
1867+
});
1868+
18061869
it("should propagate inner whenAll failure to outer whenAny in nested composites", async () => {
18071870
const hello = (_: any, name: string) => {
18081871
return `Hello ${name}!`;

0 commit comments

Comments
 (0)