Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions packages/durabletask-js/src/testing/in-memory-backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,24 +337,29 @@ export class InMemoryOrchestrationBackend {
}

return new Promise((resolve, reject) => {
const timer = setTimeout(() => {
const waiters = this.stateWaiters.get(instanceId);
if (waiters) {
const index = waiters.findIndex((w) => w.resolve === resolve);
if (index >= 0) {
waiters.splice(index, 1);
// When timeoutMs is 0, no timeout is applied — the waiter will only be
// resolved by a matching state change or rejected by reset().
let timer: ReturnType<typeof setTimeout> | undefined;
Comment on lines +340 to +342
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timeoutMs=0 now means “no timeout”. Since this is a non-obvious sentinel value (and differs from common “0 means immediate timeout” expectations), it would be good to reflect it in the waitForState JSDoc/parameter description so callers don’t accidentally create an unbounded wait.

Copilot uses AI. Check for mistakes.
if (timeoutMs > 0) {
timer = setTimeout(() => {
const waiters = this.stateWaiters.get(instanceId);
if (waiters) {
const index = waiters.findIndex((w) => w.resolve === resolve);
if (index >= 0) {
waiters.splice(index, 1);
}
}
}
reject(new Error(`Timeout waiting for orchestration '${instanceId}'`));
}, timeoutMs);
reject(new Error(`Timeout waiting for orchestration '${instanceId}'`));
Comment on lines +344 to +352
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timeout cleanup logic won’t remove the correct waiter: waiters.findIndex((w) => w.resolve === resolve) compares the waiter wrapper function to the Promise’s resolve, so the index will be -1. This leaves timed-out waiters in stateWaiters, which can cause memory leaks and later resolve() calls on an already-settled promise. Consider tracking/removing the waiter by reference (e.g., capture waiter in the timeout callback and findIndex((w) => w === waiter)) and rejecting via waiter.reject(...) so cleanup is centralized.

Copilot uses AI. Check for mistakes.
}, timeoutMs);
}

const waiter: StateWaiter = {
resolve: (result) => {
clearTimeout(timer);
if (timer !== undefined) clearTimeout(timer);
resolve(result);
},
reject: (error) => {
clearTimeout(timer);
if (timer !== undefined) clearTimeout(timer);
reject(error);
},
predicate,
Expand Down Expand Up @@ -590,8 +595,7 @@ export class InMemoryOrchestrationBackend {
this.waitForState(
subInstanceId,
(inst) => this.isTerminalStatus(inst.status),
// No timeout - sub-orchestration will eventually complete, fail, or be terminated
// If parent is terminated, we check that when delivering the event
0, // No timeout — sub-orchestration will eventually complete, fail, or be terminated
)
Comment on lines 595 to 599
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that waitForState(..., 0) can’t time out, the .catch comment below that mentions “Timeout” is misleading. Consider updating the comment to reflect the only expected cancellation path here (backend reset), or explicitly note any other errors you intend to swallow.

Copilot uses AI. Check for mistakes.
.then((subInstance) => {
const parentInstance = this.instances.get(parentInstanceId);
Expand Down
86 changes: 86 additions & 0 deletions packages/durabletask-js/test/in-memory-backend.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,56 @@ describe("In-Memory Backend", () => {
expect(activityCounter).toEqual(1);
});

it("should handle sub-orchestrations with timer delays", async () => {
const childWithTimer: TOrchestrator = async function* (ctx: OrchestrationContext): any {
// Sub-orchestration uses a short timer before returning a result
yield ctx.createTimer(0.1);
return "child-done";
};

const parentOrchestrator: TOrchestrator = async function* (ctx: OrchestrationContext): any {
const result = yield ctx.callSubOrchestrator(childWithTimer);
return `parent-received-${result}`;
};

worker.addOrchestrator(childWithTimer);
worker.addOrchestrator(parentOrchestrator);
await worker.start();

const id = await client.scheduleNewOrchestration(parentOrchestrator);
const state = await client.waitForOrchestrationCompletion(id, true, 10);

expect(state).toBeDefined();
expect(state?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED);
expect(state?.serializedOutput).toEqual(JSON.stringify("parent-received-child-done"));
Comment on lines +150 to +170
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn’t exercise the original regression (the unintended 30s timeout), because the child timer is only 0.1s. Consider using Jest fake timers to advance past 30s (e.g., a child createTimer(31)), and assert the parent still receives the completion/failure event—this would fail on the pre-fix behavior but run quickly.

Suggested change
const childWithTimer: TOrchestrator = async function* (ctx: OrchestrationContext): any {
// Sub-orchestration uses a short timer before returning a result
yield ctx.createTimer(0.1);
return "child-done";
};
const parentOrchestrator: TOrchestrator = async function* (ctx: OrchestrationContext): any {
const result = yield ctx.callSubOrchestrator(childWithTimer);
return `parent-received-${result}`;
};
worker.addOrchestrator(childWithTimer);
worker.addOrchestrator(parentOrchestrator);
await worker.start();
const id = await client.scheduleNewOrchestration(parentOrchestrator);
const state = await client.waitForOrchestrationCompletion(id, true, 10);
expect(state).toBeDefined();
expect(state?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED);
expect(state?.serializedOutput).toEqual(JSON.stringify("parent-received-child-done"));
jest.useFakeTimers();
try {
const childWithTimer: TOrchestrator = async function* (ctx: OrchestrationContext): any {
// Sub-orchestration uses a timer longer than 30s before returning a result
yield ctx.createTimer(31);
return "child-done";
};
const parentOrchestrator: TOrchestrator = async function* (ctx: OrchestrationContext): any {
const result = yield ctx.callSubOrchestrator(childWithTimer);
return `parent-received-${result}`;
};
worker.addOrchestrator(childWithTimer);
worker.addOrchestrator(parentOrchestrator);
await worker.start();
const id = await client.scheduleNewOrchestration(parentOrchestrator);
const waitPromise = client.waitForOrchestrationCompletion(id, true, 40);
// Advance fake timers past the 31s child timer to ensure the parent observes completion
jest.advanceTimersByTime(31_000);
const state = await waitPromise;
expect(state).toBeDefined();
expect(state?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED);
expect(state?.serializedOutput).toEqual(JSON.stringify("parent-received-child-done"));
} finally {
jest.useRealTimers();
}

Copilot uses AI. Check for mistakes.
});

it("should handle sub-orchestration failure", async () => {
const failingChild: TOrchestrator = async (_ctx: OrchestrationContext) => {
throw new Error("child failed");
};

const parentOrchestrator: TOrchestrator = async function* (ctx: OrchestrationContext): any {
try {
yield ctx.callSubOrchestrator(failingChild);
return "should not reach";
} catch (error: any) {
return `caught: ${error.message}`;
}
};

worker.addOrchestrator(failingChild);
worker.addOrchestrator(parentOrchestrator);
await worker.start();

const id = await client.scheduleNewOrchestration(parentOrchestrator);
const state = await client.waitForOrchestrationCompletion(id, true, 10);

expect(state).toBeDefined();
expect(state?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED);
expect(state?.serializedOutput).toContain("caught:");
});

it("should handle external events", async () => {
const orchestrator: TOrchestrator = async function* (ctx: OrchestrationContext): any {
const value = yield ctx.waitForExternalEvent("my_event");
Expand Down Expand Up @@ -351,4 +401,40 @@ describe("In-Memory Backend", () => {
expect(state?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED);
expect(state?.serializedOutput).toEqual(JSON.stringify(42));
});

it("waitForState with zero timeout should wait indefinitely until state matches", async () => {
const orchestrator: TOrchestrator = async (_: OrchestrationContext) => "done";

worker.addOrchestrator(orchestrator);
await worker.start();

const id = await client.scheduleNewOrchestration(orchestrator);

// Use waitForState with timeoutMs=0 (no timeout).
// The orchestration completes quickly, so this should resolve.
const instance = await backend.waitForState(
id,
(inst) => backend.toClientStatus(inst.status) === OrchestrationStatus.COMPLETED,
0,
);

expect(instance).toBeDefined();
});

it("waitForState with zero timeout should be rejected on reset", async () => {
// Create an instance that won't complete (no worker started)
backend.createInstance("stuck-instance", "test", JSON.stringify("input"));

// Start waiting with no timeout
const waitPromise = backend.waitForState(
"stuck-instance",
() => false, // Never matches
0,
);

// Reset should reject the waiter
backend.reset();

await expect(waitPromise).rejects.toThrow("Backend was reset");
});
});
Loading