From d2db67e6931b6cea778781e75e31e2da161f1ee5 Mon Sep 17 00:00:00 2001 From: Logan Ye Date: Wed, 29 Apr 2026 19:16:37 +0800 Subject: [PATCH] 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 --- src/gateway/server-methods/cron.ts | 36 +++++++- .../server-methods/cron.validation.test.ts | 85 +++++++++++++++++++ 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/src/gateway/server-methods/cron.ts b/src/gateway/server-methods/cron.ts index 51abf6b6296..ebbf8d63046 100644 --- a/src/gateway/server-methods/cron.ts +++ b/src/gateway/server-methods/cron.ts @@ -241,7 +241,23 @@ export const cronHandlers: GatewayRequestHandlers = { ); return; } - const job = await context.cron.add(jobCreate); + let job: Awaited>; + 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>; + 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); }, diff --git a/src/gateway/server-methods/cron.validation.test.ts b/src/gateway/server-methods/cron.validation.test.ts index 9a0922a7abc..0333a266c50 100644 --- a/src/gateway/server-methods/cron.validation.test.ts +++ b/src/gateway/server-methods/cron.validation.test.ts @@ -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(); + }); });