Skip to content

Commit 208446d

Browse files
CopilotYunchuWang
authored andcommitted
[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>
1 parent 5cc8dd0 commit 208446d

3 files changed

Lines changed: 51 additions & 2 deletions

File tree

packages/durabletask-js/src/testing/in-memory-backend.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ export class InMemoryOrchestrationBackend {
256256
): void {
257257
const instance = this.instances.get(instanceId);
258258
if (!instance) {
259-
throw new Error(`Orchestration instance '${instanceId}' not found`);
259+
return; // Instance may have been purged or the backend reset
260260
}
261261

262262
if (instance.completionToken !== completionToken) {

packages/durabletask-js/src/testing/test-worker.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,12 @@ export class TestOrchestrationWorker {
155155
failureDetails,
156156
);
157157

158-
this.backend.completeOrchestration(instanceId, completionToken, [failAction]);
158+
try {
159+
this.backend.completeOrchestration(instanceId, completionToken, [failAction]);
160+
} catch {
161+
// Instance may have been purged or the backend reset during processing.
162+
// Nothing more we can do — the orchestration result is lost.
163+
}
159164
}
160165
}
161166

packages/durabletask-js/test/in-memory-backend.spec.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,50 @@ describe("In-Memory Backend", () => {
473473
expect((backend as any).instanceTimers.has(id1)).toBe(false);
474474
});
475475

476+
it("should continue processing after backend reset during orchestration execution", async () => {
477+
// This test verifies that the worker's processing loop survives a backend
478+
// reset that occurs while an orchestration is being processed. When the
479+
// instance is deleted (e.g., via reset) before completeOrchestration runs,
480+
// the worker must not crash. Without proper handling, the processing loop
481+
// would terminate, preventing any subsequent orchestrations from running.
482+
483+
const validOrchestrator: TOrchestrator = async (_: OrchestrationContext, input: number) => {
484+
return input + 1;
485+
};
486+
487+
worker.addOrchestrator(validOrchestrator);
488+
await worker.start();
489+
490+
// Schedule and complete a first orchestration to verify baseline behavior
491+
const id1 = await client.scheduleNewOrchestration(validOrchestrator, 10);
492+
const state1 = await client.waitForOrchestrationCompletion(id1, true, 10);
493+
expect(state1?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED);
494+
expect(state1?.serializedOutput).toEqual(JSON.stringify(11));
495+
496+
// Reset the backend (simulates clearing all state while worker is running)
497+
backend.reset();
498+
499+
// Schedule a second orchestration after reset — this verifies the worker
500+
// is still alive and can process new work items
501+
const id2 = await client.scheduleNewOrchestration(validOrchestrator, 41);
502+
const state2 = await client.waitForOrchestrationCompletion(id2, true, 10);
503+
504+
expect(state2).toBeDefined();
505+
expect(state2?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED);
506+
expect(state2?.serializedOutput).toEqual(JSON.stringify(42));
507+
});
508+
509+
it("should silently ignore completeOrchestration for purged instances", () => {
510+
// Verifies that completeOrchestration returns silently when the instance
511+
// has been deleted (e.g., via purge or reset), consistent with how
512+
// completeActivity handles missing instances.
513+
514+
// Should not throw — instance simply doesn't exist
515+
expect(() => {
516+
backend.completeOrchestration("nonexistent-instance", 1, []);
517+
}).not.toThrow();
518+
});
519+
476520
it("should allow reusing instance IDs after reset", async () => {
477521
const orchestrator: TOrchestrator = async (_: OrchestrationContext, input: number) => {
478522
return input * 2;

0 commit comments

Comments
 (0)