Skip to content

Commit f2112c4

Browse files
committed
Fix retry handler treating undefined/null/NaN/Infinity as retry signal
The tryHandleRetry method used a negative check (retryResult !== false) to determine if a custom retry handler wanted to retry. This treated any non-false value — including undefined, null, NaN, and Infinity — as a retry signal, causing infinite retry loops or invalid timer creation. Changed to a positive check that only accepts explicit true or finite numbers: retryResult === true || (typeof retryResult === 'number' && Number.isFinite(retryResult)). Added 6 new unit tests covering: undefined return, null return, positive delay (number), zero delay, NaN, and Infinity. Fixes #140 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9bdc0b2 commit f2112c4

2 files changed

Lines changed: 136 additions & 3 deletions

File tree

packages/durabletask-js/src/worker/orchestration-executor.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -792,16 +792,20 @@ export class OrchestrationExecutor {
792792
task.recordFailure(errorMessage, failureDetails);
793793
const retryResult = await task.shouldRetry(ctx._currentUtcDatetime);
794794

795-
if (retryResult !== false) {
795+
// Only retry when the handler explicitly returns true or a finite number.
796+
// Using a positive check (=== true || finite number) instead of !== false
797+
// ensures that undefined/null (e.g., from a missing return statement) is
798+
// treated as "don't retry" rather than causing an infinite retry loop.
799+
if (retryResult === true || (typeof retryResult === "number" && Number.isFinite(retryResult))) {
796800
WorkerLogs.retryingTask(this._logger, ctx._instanceId, task.taskName, task.attemptCount);
797801
task.incrementAttemptCount();
798802

799803
if (typeof retryResult === "number") {
800804
if (retryResult <= 0) {
801-
// Handler returned true — retry immediately
805+
// Zero or negative delay — retry immediately
802806
ctx.rescheduleRetryTask(task);
803807
} else {
804-
// Handler returned a delay in milliseconds — use a timer
808+
// Positive delay in milliseconds — use a timer
805809
ctx.createRetryTimer(task, retryResult);
806810
}
807811
} else {

packages/durabletask-js/test/orchestration_executor.spec.ts

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,135 @@ describe("Orchestration Executor", () => {
11271127
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
11281128
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
11291129
});
1130+
1131+
it("should fail the task (not retry) when retry handler returns undefined", async () => {
1132+
const { result: startResult, replay } = await startOrchestration(
1133+
async function* (ctx: OrchestrationContext): any {
1134+
// Cast to bypass TypeScript — simulates a JavaScript consumer or a handler
1135+
// with a code path that omits a return statement
1136+
const retryHandler = (async (_retryCtx: any) => {
1137+
// Intentionally missing return statement
1138+
}) as any;
1139+
return yield ctx.callActivity("flakyActivity", undefined, { retry: retryHandler });
1140+
},
1141+
);
1142+
1143+
expect(startResult.actions[0].hasScheduletask()).toBe(true);
1144+
1145+
// Activity fails → handler returns undefined → should NOT retry, task should fail
1146+
const result = await replay(
1147+
[newTaskScheduledEvent(1, "flakyActivity")],
1148+
[newTaskFailedEvent(1, new Error("Activity error"))],
1149+
);
1150+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1151+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1152+
});
1153+
1154+
it("should fail the task (not retry) when retry handler returns null", async () => {
1155+
const { result: startResult, replay } = await startOrchestration(
1156+
async function* (ctx: OrchestrationContext): any {
1157+
const retryHandler = async (_retryCtx: any) => {
1158+
return null as any;
1159+
};
1160+
return yield ctx.callActivity("flakyActivity", undefined, { retry: retryHandler });
1161+
},
1162+
);
1163+
1164+
expect(startResult.actions[0].hasScheduletask()).toBe(true);
1165+
1166+
// Activity fails → handler returns null → should NOT retry, task should fail
1167+
const result = await replay(
1168+
[newTaskScheduledEvent(1, "flakyActivity")],
1169+
[newTaskFailedEvent(1, new Error("Activity error"))],
1170+
);
1171+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1172+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1173+
});
1174+
1175+
it("should create a retry timer when retry handler returns a positive delay", async () => {
1176+
const { result: startResult, replay } = await startOrchestration(
1177+
async function* (ctx: OrchestrationContext): any {
1178+
const retryHandler = async (_retryCtx: any) => {
1179+
return 5000; // Retry after 5 seconds
1180+
};
1181+
return yield ctx.callActivity("flakyActivity", undefined, { retry: retryHandler });
1182+
},
1183+
);
1184+
1185+
expect(startResult.actions[0].hasScheduletask()).toBe(true);
1186+
1187+
// Activity fails → handler returns 5000 → should create a timer
1188+
const result = await replay(
1189+
[newTaskScheduledEvent(1, "flakyActivity")],
1190+
[newTaskFailedEvent(1, new Error("Transient failure"))],
1191+
);
1192+
expect(result.actions.length).toBe(1);
1193+
expect(result.actions[0].hasCreatetimer()).toBe(true);
1194+
});
1195+
1196+
it("should retry immediately when retry handler returns 0", async () => {
1197+
const { result: startResult, replay } = await startOrchestration(
1198+
async function* (ctx: OrchestrationContext): any {
1199+
const retryHandler = async (_retryCtx: any) => {
1200+
return 0; // Retry immediately via zero delay
1201+
};
1202+
return yield ctx.callActivity("flakyActivity", undefined, { retry: retryHandler });
1203+
},
1204+
);
1205+
1206+
expect(startResult.actions[0].hasScheduletask()).toBe(true);
1207+
1208+
// Activity fails → handler returns 0 → should reschedule immediately (no timer)
1209+
const result = await replay(
1210+
[newTaskScheduledEvent(1, "flakyActivity")],
1211+
[newTaskFailedEvent(1, new Error("Transient failure"))],
1212+
);
1213+
expect(result.actions.length).toBe(1);
1214+
expect(result.actions[0].hasScheduletask()).toBe(true);
1215+
expect(result.actions[0].getScheduletask()?.getName()).toBe("flakyActivity");
1216+
});
1217+
1218+
it("should fail the task when retry handler returns NaN", async () => {
1219+
const { result: startResult, replay } = await startOrchestration(
1220+
async function* (ctx: OrchestrationContext): any {
1221+
const retryHandler = async (_retryCtx: any) => {
1222+
return NaN;
1223+
};
1224+
return yield ctx.callActivity("flakyActivity", undefined, { retry: retryHandler });
1225+
},
1226+
);
1227+
1228+
expect(startResult.actions[0].hasScheduletask()).toBe(true);
1229+
1230+
// Activity fails → handler returns NaN → should NOT retry
1231+
const result = await replay(
1232+
[newTaskScheduledEvent(1, "flakyActivity")],
1233+
[newTaskFailedEvent(1, new Error("Activity error"))],
1234+
);
1235+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1236+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1237+
});
1238+
1239+
it("should fail the task when retry handler returns Infinity", async () => {
1240+
const { result: startResult, replay } = await startOrchestration(
1241+
async function* (ctx: OrchestrationContext): any {
1242+
const retryHandler = async (_retryCtx: any) => {
1243+
return Infinity;
1244+
};
1245+
return yield ctx.callActivity("flakyActivity", undefined, { retry: retryHandler });
1246+
},
1247+
);
1248+
1249+
expect(startResult.actions[0].hasScheduletask()).toBe(true);
1250+
1251+
// Activity fails → handler returns Infinity → should NOT retry
1252+
const result = await replay(
1253+
[newTaskScheduledEvent(1, "flakyActivity")],
1254+
[newTaskFailedEvent(1, new Error("Activity error"))],
1255+
);
1256+
const completeAction = getAndValidateSingleCompleteOrchestrationAction(result);
1257+
expect(completeAction?.getOrchestrationstatus()).toEqual(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED);
1258+
});
11301259
});
11311260

11321261
it("should complete immediately when whenAll is called with an empty task array", async () => {

0 commit comments

Comments
 (0)