From e9ce8b9e4e84119afc174137af1149ab1ec151b4 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 20:01:45 +0000 Subject: [PATCH 1/2] [copilot-finds] Bug: TestOrchestrationWorker processing loop crashes when completeOrchestration fails completeOrchestration throws when the instance has been purged or the backend reset, but completeActivity silently returns for the same case. When the test worker's processOrchestration catch block calls completeOrchestration on a missing instance, the unhandled exception propagates to runProcessingLoop and kills it. All subsequent work items are silently dropped. Fix both the root cause and the symptom: - completeOrchestration now returns silently for missing instances, consistent with completeActivity - processOrchestration's catch block wraps completeOrchestration in a nested try-catch for defense-in-depth Add two new tests: - Worker continues processing after backend reset during execution - completeOrchestration silently ignores purged instances Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/testing/in-memory-backend.ts | 2 +- .../durabletask-js/src/testing/test-worker.ts | 7 ++- .../test/in-memory-backend.spec.ts | 44 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/packages/durabletask-js/src/testing/in-memory-backend.ts b/packages/durabletask-js/src/testing/in-memory-backend.ts index 66cb406..f693061 100644 --- a/packages/durabletask-js/src/testing/in-memory-backend.ts +++ b/packages/durabletask-js/src/testing/in-memory-backend.ts @@ -256,7 +256,7 @@ export class InMemoryOrchestrationBackend { ): void { const instance = this.instances.get(instanceId); if (!instance) { - throw new Error(`Orchestration instance '${instanceId}' not found`); + return; // Instance may have been purged or the backend reset } if (instance.completionToken !== completionToken) { diff --git a/packages/durabletask-js/src/testing/test-worker.ts b/packages/durabletask-js/src/testing/test-worker.ts index 09855c6..17a564c 100644 --- a/packages/durabletask-js/src/testing/test-worker.ts +++ b/packages/durabletask-js/src/testing/test-worker.ts @@ -155,7 +155,12 @@ export class TestOrchestrationWorker { failureDetails, ); - this.backend.completeOrchestration(instanceId, completionToken, [failAction]); + try { + this.backend.completeOrchestration(instanceId, completionToken, [failAction]); + } catch { + // Instance may have been purged or the backend reset during processing. + // Nothing more we can do — the orchestration result is lost. + } } } diff --git a/packages/durabletask-js/test/in-memory-backend.spec.ts b/packages/durabletask-js/test/in-memory-backend.spec.ts index 81a82a8..2029d2b 100644 --- a/packages/durabletask-js/test/in-memory-backend.spec.ts +++ b/packages/durabletask-js/test/in-memory-backend.spec.ts @@ -473,6 +473,50 @@ describe("In-Memory Backend", () => { expect((backend as any).instanceTimers.has(id1)).toBe(false); }); + it("should continue processing after backend reset during orchestration execution", async () => { + // This test verifies that the worker's processing loop survives a backend + // reset that occurs while an orchestration is being processed. When the + // instance is deleted (e.g., via reset) before completeOrchestration runs, + // the worker must not crash. Without proper handling, the processing loop + // would terminate, preventing any subsequent orchestrations from running. + + const validOrchestrator: TOrchestrator = async (_: OrchestrationContext, input: number) => { + return input + 1; + }; + + worker.addOrchestrator(validOrchestrator); + await worker.start(); + + // Schedule and complete a first orchestration to verify baseline behavior + const id1 = await client.scheduleNewOrchestration(validOrchestrator, 10); + const state1 = await client.waitForOrchestrationCompletion(id1, true, 10); + expect(state1?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED); + expect(state1?.serializedOutput).toEqual(JSON.stringify(11)); + + // Reset the backend (simulates clearing all state while worker is running) + backend.reset(); + + // Schedule a second orchestration after reset — this verifies the worker + // is still alive and can process new work items + const id2 = await client.scheduleNewOrchestration(validOrchestrator, 41); + const state2 = await client.waitForOrchestrationCompletion(id2, true, 10); + + expect(state2).toBeDefined(); + expect(state2?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED); + expect(state2?.serializedOutput).toEqual(JSON.stringify(42)); + }); + + it("should silently ignore completeOrchestration for purged instances", () => { + // Verifies that completeOrchestration returns silently when the instance + // has been deleted (e.g., via purge or reset), consistent with how + // completeActivity handles missing instances. + + // Should not throw — instance simply doesn't exist + expect(() => { + backend.completeOrchestration("nonexistent-instance", 1, []); + }).not.toThrow(); + }); + it("should allow reusing instance IDs after reset", async () => { const orchestrator: TOrchestrator = async (_: OrchestrationContext, input: number) => { return input * 2; From ae6faa9b6e6d83b2da3f85d09b5c9e3c29a8c033 Mon Sep 17 00:00:00 2001 From: wangbill Date: Fri, 17 Jul 2026 11:50:09 -0700 Subject: [PATCH 2/2] [copilot-finds] Bug: keep only the root fix for test-worker completion race (#174) Simplify the fix for the TestOrchestrationWorker processing-loop crash down to the minimal necessary change: InMemoryOrchestrationBackend.completeOrchestration() returns a no-op when the target instance is missing (purged/reset), mirroring completeActivity(). Drop two changes flagged in review: - Remove the blanket try/catch around the failure-path completeOrchestration call in test-worker.ts. With the root fix a missing instance no longer throws, so the catch only served to swallow real errors (e.g. history-size limit), which could hide backend bugs and cause silent hangs. - Remove the "continue processing after backend reset during orchestration execution" test. It reset the backend only after the first orchestration had fully completed, so it never reproduced the #174 race and passed even without the fix. Keep the "silently ignore completeOrchestration for purged instances" test, which deterministically validates the root fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d175a43f-c852-49a2-a0de-63db579a8e3a --- .../durabletask-js/src/testing/test-worker.ts | 7 +--- .../test/in-memory-backend.spec.ts | 33 ------------------- 2 files changed, 1 insertion(+), 39 deletions(-) diff --git a/packages/durabletask-js/src/testing/test-worker.ts b/packages/durabletask-js/src/testing/test-worker.ts index 17a564c..09855c6 100644 --- a/packages/durabletask-js/src/testing/test-worker.ts +++ b/packages/durabletask-js/src/testing/test-worker.ts @@ -155,12 +155,7 @@ export class TestOrchestrationWorker { failureDetails, ); - try { - this.backend.completeOrchestration(instanceId, completionToken, [failAction]); - } catch { - // Instance may have been purged or the backend reset during processing. - // Nothing more we can do — the orchestration result is lost. - } + this.backend.completeOrchestration(instanceId, completionToken, [failAction]); } } diff --git a/packages/durabletask-js/test/in-memory-backend.spec.ts b/packages/durabletask-js/test/in-memory-backend.spec.ts index 2029d2b..2ee1028 100644 --- a/packages/durabletask-js/test/in-memory-backend.spec.ts +++ b/packages/durabletask-js/test/in-memory-backend.spec.ts @@ -473,39 +473,6 @@ describe("In-Memory Backend", () => { expect((backend as any).instanceTimers.has(id1)).toBe(false); }); - it("should continue processing after backend reset during orchestration execution", async () => { - // This test verifies that the worker's processing loop survives a backend - // reset that occurs while an orchestration is being processed. When the - // instance is deleted (e.g., via reset) before completeOrchestration runs, - // the worker must not crash. Without proper handling, the processing loop - // would terminate, preventing any subsequent orchestrations from running. - - const validOrchestrator: TOrchestrator = async (_: OrchestrationContext, input: number) => { - return input + 1; - }; - - worker.addOrchestrator(validOrchestrator); - await worker.start(); - - // Schedule and complete a first orchestration to verify baseline behavior - const id1 = await client.scheduleNewOrchestration(validOrchestrator, 10); - const state1 = await client.waitForOrchestrationCompletion(id1, true, 10); - expect(state1?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED); - expect(state1?.serializedOutput).toEqual(JSON.stringify(11)); - - // Reset the backend (simulates clearing all state while worker is running) - backend.reset(); - - // Schedule a second orchestration after reset — this verifies the worker - // is still alive and can process new work items - const id2 = await client.scheduleNewOrchestration(validOrchestrator, 41); - const state2 = await client.waitForOrchestrationCompletion(id2, true, 10); - - expect(state2).toBeDefined(); - expect(state2?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED); - expect(state2?.serializedOutput).toEqual(JSON.stringify(42)); - }); - it("should silently ignore completeOrchestration for purged instances", () => { // Verifies that completeOrchestration returns silently when the instance // has been deleted (e.g., via purge or reset), consistent with how