From 97c76fac80cb761b627e2af8f01f06195f8a52fe Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 17:38:25 +0000 Subject: [PATCH] [copilot-finds] Bug: Missing non-Task validation in resume() isFailed path When an orchestrator catches an exception from a failed task and yields a non-Task value, the runtime silently falls through without reporting an error. This leaves _previousTask pointing to the old failed task, corrupting the generator state. On the next resume() call, the same exception is thrown to the generator again in an unexpected location. The isComplete path already validates this (line 184-186) and throws a clear error: 'The orchestrator generator yielded a non-Task object'. The isFailed path lacked this same validation. This fix adds the same non-Task validation to the isFailed path, ensuring consistent behavior and a clear error message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../worker/runtime-orchestration-context.ts | 4 + .../test/orchestration_executor.spec.ts | 84 +++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/packages/durabletask-js/src/worker/runtime-orchestration-context.ts b/packages/durabletask-js/src/worker/runtime-orchestration-context.ts index a14f5e2..9dd1332 100644 --- a/packages/durabletask-js/src/worker/runtime-orchestration-context.ts +++ b/packages/durabletask-js/src/worker/runtime-orchestration-context.ts @@ -164,6 +164,10 @@ export class RuntimeOrchestrationContext extends OrchestrationContext { } return; } + + // The generator yielded a non-Task value after catching the exception. + // This is a user error — orchestrators must only yield Task objects. + throw new Error("The orchestrator generator yielded a non-Task object"); } else if (this._previousTask.isComplete) { while (true) { // Resume the generator. This will either return a Task or raise StopIteration if it's done. diff --git a/packages/durabletask-js/test/orchestration_executor.spec.ts b/packages/durabletask-js/test/orchestration_executor.spec.ts index c4cd604..86643a6 100644 --- a/packages/durabletask-js/test/orchestration_executor.spec.ts +++ b/packages/durabletask-js/test/orchestration_executor.spec.ts @@ -246,6 +246,90 @@ describe("Orchestration Executor", () => { // const userCodeStatement = "ctx.callActivity(dummyActivity, orchestratorInput)"; // expect(completeAction?.getFailuredetails()?.getStacktrace()?.getValue()).toContain(userCodeStatement); }); + it("should fail the orchestration when the generator yields a non-Task value after catching an exception", async () => { + const dummyActivity = async (_: ActivityContext) => { + // do nothing + }; + // This orchestrator catches the exception and yields a non-Task value (a string). + // The runtime should detect this and fail the orchestration with a clear error message, + // matching the validation already present in the isComplete path. + const orchestrator: TOrchestrator = async function* (ctx: OrchestrationContext): any { + try { + yield ctx.callActivity(dummyActivity); + } catch { + yield "not a task" as any; + } + }; + const registry = new Registry(); + const orchestratorName = registry.addOrchestrator(orchestrator); + const activityName = registry.addActivity(dummyActivity); + const oldEvents = [ + newOrchestratorStartedEvent(), + newExecutionStartedEvent(orchestratorName, TEST_INSTANCE_ID, undefined), + newTaskScheduledEvent(1, activityName), + ]; + const ex = new Error("Activity failed"); + const newEvents = [newTaskFailedEvent(1, ex)]; + const executor = new OrchestrationExecutor(registry, testLogger); + const result = await executor.execute(TEST_INSTANCE_ID, oldEvents, newEvents); + const completeAction = getAndValidateSingleCompleteOrchestrationAction(result); + expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + expect(completeAction?.getFailuredetails()?.getErrormessage()).toContain("non-Task"); + }); + + it("should complete successfully when the generator catches an exception and yields a new Task", async () => { + const failingActivity = async (_: ActivityContext) => { + throw new Error("fail"); + }; + const successActivity = async (_: ActivityContext) => { + return "ok"; + }; + // This orchestrator catches the exception from the first activity and + // yields a new activity call. The runtime should process the new task correctly. + const orchestrator: TOrchestrator = async function* (ctx: OrchestrationContext): any { + try { + yield ctx.callActivity(failingActivity); + } catch { + const result = yield ctx.callActivity(successActivity); + return result; + } + }; + const registry = new Registry(); + const orchestratorName = registry.addOrchestrator(orchestrator); + const failingName = registry.addActivity(failingActivity); + const successName = registry.addActivity(successActivity); + + // First execution: activity fails, orchestrator catches and schedules a new activity + const oldEvents = [ + newOrchestratorStartedEvent(), + newExecutionStartedEvent(orchestratorName, TEST_INSTANCE_ID, undefined), + newTaskScheduledEvent(1, failingName), + ]; + const ex = new Error("Activity failed"); + const newEvents = [newTaskFailedEvent(1, ex)]; + const executor = new OrchestrationExecutor(registry, testLogger); + let result = await executor.execute(TEST_INSTANCE_ID, oldEvents, newEvents); + + // Should have scheduled a new activity (scheduleTask action) + expect(result.actions.length).toEqual(1); + expect(result.actions[0].hasScheduletask()).toBe(true); + + // Second execution: replay + new activity completes + const replayEvents = [ + newOrchestratorStartedEvent(), + newExecutionStartedEvent(orchestratorName, TEST_INSTANCE_ID, undefined), + newTaskScheduledEvent(1, failingName), + newTaskFailedEvent(1, ex), + newTaskScheduledEvent(2, successName), + ]; + const completionEvents = [newTaskCompletedEvent(2, JSON.stringify("ok"))]; + const executor2 = new OrchestrationExecutor(registry, testLogger); + result = await executor2.execute(TEST_INSTANCE_ID, replayEvents, completionEvents); + const completeAction = getAndValidateSingleCompleteOrchestrationAction(result); + expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_COMPLETED); + expect(completeAction?.getResult()?.getValue()).toEqual(JSON.stringify("ok")); + }); + it("should test the non-determinism detection logic when callTimer is expected but some other method (callActivity) is called instead", async () => { const dummyActivity = async (_: ActivityContext) => { // do nothing