fix(cron): catch croner parse errors in cron.add and cron.update handlers (#74193)

* fix(cron): catch croner parse errors in cron.add and cron.update handlers

* fix(cron): narrow catch to TypeError/RangeError only; add braces for linter
This commit is contained in:
Logan Ye
2026-04-29 19:16:37 +08:00
committed by GitHub
parent 2aa6abddbe
commit d2db67e693
2 changed files with 119 additions and 2 deletions

View File

@@ -241,7 +241,23 @@ export const cronHandlers: GatewayRequestHandlers = {
);
return;
}
const job = await context.cron.add(jobCreate);
let job: Awaited<ReturnType<typeof context.cron.add>>;
try {
job = await context.cron.add(jobCreate);
} catch (err) {
if (!(err instanceof TypeError) && !(err instanceof RangeError)) {
throw err;
}
respond(
false,
undefined,
errorShape(
ErrorCodes.INVALID_REQUEST,
`invalid cron.add params: ${formatErrorMessage(err)}`,
),
);
return;
}
context.logGateway.info("cron: job created", { jobId: job.id, schedule: jobCreate.schedule });
respond(true, job, undefined);
},
@@ -320,7 +336,23 @@ export const cronHandlers: GatewayRequestHandlers = {
);
return;
}
const job = await context.cron.update(jobId, patch);
let job: Awaited<ReturnType<typeof context.cron.update>>;
try {
job = await context.cron.update(jobId, patch);
} catch (err) {
if (!(err instanceof TypeError) && !(err instanceof RangeError)) {
throw err;
}
respond(
false,
undefined,
errorShape(
ErrorCodes.INVALID_REQUEST,
`invalid cron.update params: ${formatErrorMessage(err)}`,
),
);
return;
}
context.logGateway.info("cron: job updated", { jobId });
respond(true, job, undefined);
},

View File

@@ -294,4 +294,89 @@ describe("cron method validation", () => {
}),
);
});
it("returns INVALID_REQUEST when cron.add throws a croner parse error (#74066)", async () => {
const context = createCronContext();
context.cron.add.mockRejectedValueOnce(new TypeError("CronPattern: Expected 5 or 6 fields"));
const respond = vi.fn();
await cronHandlers["cron.add"]({
req: {} as never,
params: {
name: "bad-cron",
enabled: true,
schedule: { kind: "cron", cron: "not-a-cron-expr" },
sessionTarget: "isolated",
wakeMode: "next-heartbeat",
payload: { kind: "agentTurn", message: "ping" },
} as never,
respond: respond as never,
context: context as never,
client: null,
isWebchatConnect: () => false,
});
expect(respond).toHaveBeenCalledWith(
false,
undefined,
expect.objectContaining({
code: "INVALID_REQUEST",
message: expect.stringContaining("CronPattern"),
}),
);
});
it("returns INVALID_REQUEST when cron.update throws a croner parse error (#74066)", async () => {
const existingJob = createCronJob();
const context = createCronContext(existingJob);
context.cron.update.mockRejectedValueOnce(
new RangeError("CronPattern: Value out of range (99)"),
);
const respond = vi.fn();
await cronHandlers["cron.update"]({
req: {} as never,
params: {
id: existingJob.id,
patch: {
schedule: { kind: "cron", cron: "99 * * * *" },
},
} as never,
respond: respond as never,
context: context as never,
client: null,
isWebchatConnect: () => false,
});
expect(respond).toHaveBeenCalledWith(
false,
undefined,
expect.objectContaining({
code: "INVALID_REQUEST",
message: expect.stringContaining("CronPattern"),
}),
);
});
it("re-throws non-parse errors from cron.add instead of masking as INVALID_REQUEST", async () => {
const context = createCronContext();
context.cron.add.mockRejectedValueOnce(new Error("DB write failed"));
const respond = vi.fn();
await expect(
cronHandlers["cron.add"]({
req: {} as never,
params: {
name: "db-fail",
enabled: true,
schedule: { kind: "every", everyMs: 60_000 },
sessionTarget: "isolated",
wakeMode: "next-heartbeat",
payload: { kind: "agentTurn", message: "ping" },
} as never,
respond: respond as never,
context: context as never,
client: null,
isWebchatConnect: () => false,
}),
).rejects.toThrow("DB write failed");
expect(respond).not.toHaveBeenCalled();
});
});