refactor: harden cron validation and fail early in case of wrong pattern
This commit is contained in:
parent
938680bb80
commit
3d2900ab69
5 changed files with 186 additions and 23 deletions
145
app/server/core/__tests__/scheduler.test.ts
Normal file
145
app/server/core/__tests__/scheduler.test.ts
Normal file
|
|
@ -0,0 +1,145 @@
|
|||
import { afterEach, describe, expect, mock, spyOn, test, vi } from "bun:test";
|
||||
import { logger } from "~/server/utils/logger";
|
||||
import { Job, Scheduler } from "../scheduler";
|
||||
|
||||
const flushMicrotasks = async () => {
|
||||
await Promise.resolve();
|
||||
};
|
||||
|
||||
const mockTimeZone = (timeZone: string) => {
|
||||
const resolvedOptions = Intl.DateTimeFormat.prototype.resolvedOptions;
|
||||
return spyOn(Intl.DateTimeFormat.prototype, "resolvedOptions").mockImplementation(
|
||||
function (this: Intl.DateTimeFormat) {
|
||||
return { ...resolvedOptions.call(this), timeZone };
|
||||
},
|
||||
);
|
||||
};
|
||||
|
||||
describe("Scheduler", () => {
|
||||
afterEach(async () => {
|
||||
await Scheduler.clear();
|
||||
mock.restore();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
test("keeps future cron ticks armed while a job is still running", async () => {
|
||||
vi.useFakeTimers({ now: new Date("2026-03-08T00:00:00.000Z") });
|
||||
|
||||
let runCount = 0;
|
||||
let releaseRun: (() => void) | undefined;
|
||||
const warn = spyOn(logger, "warn");
|
||||
|
||||
class TestJob extends Job {
|
||||
async run() {
|
||||
runCount += 1;
|
||||
await new Promise<void>((resolve) => {
|
||||
releaseRun = resolve;
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Scheduler.build(TestJob).schedule("* * * * *");
|
||||
|
||||
expect(vi.getTimerCount()).toBe(1);
|
||||
|
||||
vi.advanceTimersByTime(60_000);
|
||||
await flushMicrotasks();
|
||||
|
||||
expect(runCount).toBe(1);
|
||||
expect(vi.getTimerCount()).toBe(1);
|
||||
|
||||
vi.advanceTimersByTime(60_000);
|
||||
await flushMicrotasks();
|
||||
|
||||
expect(runCount).toBe(1);
|
||||
expect(warn).toHaveBeenCalledWith("Skipping overlapping run for job TestJob");
|
||||
expect(vi.getTimerCount()).toBe(1);
|
||||
|
||||
releaseRun?.();
|
||||
await flushMicrotasks();
|
||||
});
|
||||
|
||||
test("throws for invalid cron expressions instead of retrying with a fallback timer", () => {
|
||||
vi.useFakeTimers({ now: new Date("2026-03-08T00:00:00.000Z") });
|
||||
|
||||
class InvalidCronJob extends Job {
|
||||
async run() {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
expect(() => Scheduler.build(InvalidCronJob).schedule("not a cron")).toThrow();
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
});
|
||||
|
||||
test("uses the local timezone instead of UTC when arming the next run", async () => {
|
||||
mockTimeZone("America/New_York");
|
||||
vi.useFakeTimers({ now: new Date("2026-01-15T04:59:00.000Z") });
|
||||
|
||||
let runCount = 0;
|
||||
|
||||
class MidnightJob extends Job {
|
||||
async run() {
|
||||
runCount += 1;
|
||||
}
|
||||
}
|
||||
|
||||
Scheduler.build(MidnightJob).schedule("0 0 * * *");
|
||||
|
||||
vi.advanceTimersByTime(59_000);
|
||||
await flushMicrotasks();
|
||||
expect(runCount).toBe(0);
|
||||
|
||||
vi.advanceTimersByTime(1_000);
|
||||
await flushMicrotasks();
|
||||
expect(runCount).toBe(1);
|
||||
});
|
||||
|
||||
test("runs skipped-hour cron expressions once when daylight saving time jumps forward", async () => {
|
||||
mockTimeZone("America/New_York");
|
||||
vi.useFakeTimers({ now: new Date("2026-03-08T06:59:00.000Z") });
|
||||
|
||||
let runCount = 0;
|
||||
|
||||
class SpringForwardJob extends Job {
|
||||
async run() {
|
||||
runCount += 1;
|
||||
}
|
||||
}
|
||||
|
||||
Scheduler.build(SpringForwardJob).schedule("0 2 * * *");
|
||||
|
||||
vi.advanceTimersByTime(60_000);
|
||||
await flushMicrotasks();
|
||||
expect(runCount).toBe(1);
|
||||
expect(vi.getTimerCount()).toBe(1);
|
||||
|
||||
vi.advanceTimersByTime(60_000);
|
||||
await flushMicrotasks();
|
||||
expect(runCount).toBe(1);
|
||||
});
|
||||
|
||||
test("does not run the same repeated local time twice when daylight saving time falls back", async () => {
|
||||
mockTimeZone("America/New_York");
|
||||
vi.useFakeTimers({ now: new Date("2026-11-01T05:20:00.000Z") });
|
||||
|
||||
let runCount = 0;
|
||||
|
||||
class FallBackJob extends Job {
|
||||
async run() {
|
||||
runCount += 1;
|
||||
}
|
||||
}
|
||||
|
||||
Scheduler.build(FallBackJob).schedule("30 1 * * *");
|
||||
|
||||
vi.advanceTimersByTime(10 * 60_000);
|
||||
await flushMicrotasks();
|
||||
expect(runCount).toBe(1);
|
||||
|
||||
vi.advanceTimersByTime(70 * 60_000);
|
||||
await flushMicrotasks();
|
||||
expect(runCount).toBe(1);
|
||||
expect(vi.getTimerCount()).toBe(1);
|
||||
});
|
||||
});
|
||||
|
|
@ -11,29 +11,39 @@ class ScheduledTask {
|
|||
private timer: ReturnType<typeof setTimeout> | null = null;
|
||||
private active = true;
|
||||
private running = false;
|
||||
private readonly timeZone = Intl.DateTimeFormat().resolvedOptions().timeZone || undefined;
|
||||
|
||||
constructor(
|
||||
private readonly jobName: string,
|
||||
private readonly cronExpression: string,
|
||||
private readonly run: () => Promise<void>,
|
||||
) {
|
||||
CronExpressionParser.parse(this.cronExpression);
|
||||
this.scheduleNext();
|
||||
}
|
||||
|
||||
private getDelay(fromDate: Date) {
|
||||
try {
|
||||
const interval = CronExpressionParser.parse(this.cronExpression, {
|
||||
currentDate: fromDate,
|
||||
tz: Intl.DateTimeFormat().resolvedOptions().timeZone,
|
||||
});
|
||||
const nextRun = interval.next().toDate();
|
||||
return Math.max(0, nextRun.getTime() - Date.now());
|
||||
} catch (error) {
|
||||
logger.error(`Failed to parse cron expression for ${this.jobName}:`, error);
|
||||
return 60_000;
|
||||
const interval = CronExpressionParser.parse(this.cronExpression, {
|
||||
currentDate: fromDate,
|
||||
tz: this.timeZone,
|
||||
});
|
||||
const nextRun = interval.next().toDate();
|
||||
return Math.max(0, nextRun.getTime() - Date.now());
|
||||
}
|
||||
|
||||
private clearTimer() {
|
||||
if (this.timer) {
|
||||
clearTimeout(this.timer);
|
||||
this.timer = null;
|
||||
}
|
||||
}
|
||||
|
||||
private stopScheduling(error: unknown) {
|
||||
this.active = false;
|
||||
this.clearTimer();
|
||||
logger.error(`Stopping scheduled job ${this.jobName} after cron parsing failed:`, error);
|
||||
}
|
||||
|
||||
private scheduleNext(fromDate = new Date()) {
|
||||
if (!this.active) return;
|
||||
|
||||
|
|
@ -45,10 +55,17 @@ class ScheduledTask {
|
|||
|
||||
private async tick() {
|
||||
if (!this.active) return;
|
||||
this.timer = null;
|
||||
|
||||
try {
|
||||
this.scheduleNext(new Date());
|
||||
} catch (error) {
|
||||
this.stopScheduling(error);
|
||||
return;
|
||||
}
|
||||
|
||||
if (this.running) {
|
||||
logger.warn(`Skipping overlapping run for job ${this.jobName}`);
|
||||
this.scheduleNext(new Date());
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -57,16 +74,12 @@ class ScheduledTask {
|
|||
await this.run();
|
||||
} finally {
|
||||
this.running = false;
|
||||
this.scheduleNext(new Date());
|
||||
}
|
||||
}
|
||||
|
||||
async stop() {
|
||||
this.active = false;
|
||||
if (this.timer) {
|
||||
clearTimeout(this.timer);
|
||||
this.timer = null;
|
||||
}
|
||||
this.clearTimer();
|
||||
}
|
||||
|
||||
async destroy() {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,15 @@
|
|||
import { and, eq, inArray } from "drizzle-orm";
|
||||
import cron from "node-cron";
|
||||
import CronExpressionParser from "cron-parser";
|
||||
import { NotFoundError, BadRequestError, ConflictError } from "http-errors-enhanced";
|
||||
|
||||
const isValidCron = (expression: string): boolean => {
|
||||
try {
|
||||
CronExpressionParser.parse(expression);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
import { db } from "../../db/db";
|
||||
import { backupScheduleMirrorsTable, backupScheduleNotificationsTable, backupSchedulesTable } from "../../db/schema";
|
||||
import type { CreateBackupScheduleBody, UpdateBackupScheduleBody, UpdateScheduleMirrorsBody } from "./backups.dto";
|
||||
|
|
@ -83,7 +92,7 @@ const getScheduleByIdOrShortId = async (idOrShortId: string | number) => {
|
|||
|
||||
const createSchedule = async (data: CreateBackupScheduleBody) => {
|
||||
const organizationId = getOrganizationId();
|
||||
if (!cron.validate(data.cronExpression)) {
|
||||
if (!isValidCron(data.cronExpression)) {
|
||||
throw new BadRequestError("Invalid cron expression");
|
||||
}
|
||||
|
||||
|
|
@ -169,7 +178,7 @@ const updateSchedule = async (scheduleIdOrShortId: number | string, data: Update
|
|||
throw new NotFoundError("Backup schedule not found");
|
||||
}
|
||||
|
||||
if (data.cronExpression && !cron.validate(data.cronExpression)) {
|
||||
if (data.cronExpression && !isValidCron(data.cronExpression)) {
|
||||
throw new BadRequestError("Invalid cron expression");
|
||||
}
|
||||
|
||||
|
|
|
|||
3
bun.lock
3
bun.lock
|
|
@ -55,7 +55,6 @@
|
|||
"isbot": "^5.1.35",
|
||||
"lucide-react": "^0.577.0",
|
||||
"next-themes": "^0.4.6",
|
||||
"node-cron": "^4.2.1",
|
||||
"qrcode.react": "^4.2.0",
|
||||
"react": "^19.2.4",
|
||||
"react-dom": "^19.2.4",
|
||||
|
|
@ -1554,8 +1553,6 @@
|
|||
|
||||
"nitro": ["nitro@3.0.1-alpha.2", "", { "dependencies": { "consola": "^3.4.2", "crossws": "^0.4.3", "db0": "^0.3.4", "h3": "^2.0.1-rc.11", "jiti": "^2.6.1", "nf3": "^0.3.5", "ofetch": "^2.0.0-alpha.3", "ohash": "^2.0.11", "oxc-minify": "^0.110.0", "oxc-transform": "^0.110.0", "srvx": "^0.10.1", "undici": "^7.18.2", "unenv": "^2.0.0-rc.24", "unstorage": "^2.0.0-alpha.5" }, "peerDependencies": { "rolldown": ">=1.0.0-beta.0", "rollup": "^4", "vite": "^7 || ^8 || >=8.0.0-0", "xml2js": "^0.6.2" }, "optionalPeers": ["rolldown", "rollup", "vite", "xml2js"], "bin": { "nitro": "dist/cli/index.mjs" } }, "sha512-YviDY5J/trS821qQ1fpJtpXWIdPYiOizC/meHavlm1Hfuhx//H+Egd1+4C5SegJRgtWMnRPW9n//6Woaw81cTQ=="],
|
||||
|
||||
"node-cron": ["node-cron@4.2.1", "", {}, "sha512-lgimEHPE/QDgFlywTd8yTR61ptugX3Qer29efeyWw2rv259HtGBNn1vZVmp8lB9uo9wC0t/AT4iGqXxia+CJFg=="],
|
||||
|
||||
"node-domexception": ["node-domexception@1.0.0", "", {}, "sha512-/jKZoMpw0F8GRwl4/eLROPA3cfcXtLApP0QzLmUT/HuPCZWyB7IY9ZrMeKw2O/nFIqPQB3PVM9aYm0F312AXDQ=="],
|
||||
|
||||
"node-fetch": ["node-fetch@3.3.2", "", { "dependencies": { "data-uri-to-buffer": "^4.0.0", "fetch-blob": "^3.1.4", "formdata-polyfill": "^4.0.10" } }, "sha512-dRB78srN/l6gqWulah9SrxeYnxeddIG30+GOqK/9OlLVyLg3HPnr6SqOWTWOXKRwC2eGYCkZ59NNuSgvSrpgOA=="],
|
||||
|
|
|
|||
|
|
@ -76,7 +76,6 @@
|
|||
"isbot": "^5.1.35",
|
||||
"lucide-react": "^0.577.0",
|
||||
"next-themes": "^0.4.6",
|
||||
"node-cron": "^4.2.1",
|
||||
"qrcode.react": "^4.2.0",
|
||||
"react": "^19.2.4",
|
||||
"react-dom": "^19.2.4",
|
||||
|
|
|
|||
Loading…
Reference in a new issue