Skip to content

Commit beb6074

Browse files
cardernecarderne
committed
cap at 24h, no d option, cap too-large windows and log
1 parent 75a11b4 commit beb6074

9 files changed

Lines changed: 88 additions & 123 deletions

File tree

‎apps/webapp/app/v3/scheduleWindow.server.ts‎

Lines changed: 5 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,9 @@
1-
import {
2-
calculateNextNominalTimestamp,
3-
parseScheduleWindow,
4-
validateScheduleWindowForInterval,
5-
} from "@internal/schedule-engine";
1+
import { parseScheduleWindow } from "@internal/schedule-engine";
62
import type { ScheduleWindow } from "@trigger.dev/core/v3";
7-
import { calculateNextScheduledTimestampFromNow } from "./utils/calculateNextSchedule.server";
83

94
const SECONDS_PER_UNIT = {
105
m: 60,
116
h: 3_600,
12-
d: 86_400,
137
} as const;
148

159
export type ScheduleWindowDatabaseFields = {
@@ -58,40 +52,22 @@ export function formatScheduleWindow({
5852
return "0m";
5953
}
6054

61-
if (windowDurationSeconds % SECONDS_PER_UNIT.d === 0) {
62-
return `${windowDurationSeconds / SECONDS_PER_UNIT.d}d`;
63-
}
64-
6555
if (windowDurationSeconds % SECONDS_PER_UNIT.h === 0) {
6656
return `${windowDurationSeconds / SECONDS_PER_UNIT.h}h`;
6757
}
6858

6959
return `${windowDurationSeconds / SECONDS_PER_UNIT.m}m`;
7060
}
7161

72-
export function validateScheduleWindowAgainstCron({
73-
window,
74-
cron,
75-
timezone,
76-
}: {
77-
window: ScheduleWindow | undefined;
78-
cron: string;
79-
timezone: string | null;
80-
}): { valid: true } | { valid: false; message: string } {
62+
export function validateScheduleWindowSyntax(
63+
window: ScheduleWindow | undefined
64+
): { valid: true } | { valid: false; message: string } {
8165
if (window === undefined) {
8266
return { valid: true };
8367
}
8468

8569
try {
86-
const normalizedWindow = parseScheduleWindow(window);
87-
const nominalAt = calculateNextScheduledTimestampFromNow(cron, timezone);
88-
const nextNominalAt = calculateNextNominalTimestamp(cron, timezone, nominalAt);
89-
90-
validateScheduleWindowForInterval(
91-
normalizedWindow,
92-
nextNominalAt.getTime() - nominalAt.getTime()
93-
);
94-
70+
parseScheduleWindow(window);
9571
return { valid: true };
9672
} catch (error) {
9773
return {

‎apps/webapp/app/v3/services/checkSchedule.server.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { getTimezones } from "~/utils/timezones.server";
77
import { env } from "~/env.server";
88
import type { ScheduleWindow } from "@trigger.dev/core/v3";
99
import { type PrismaClientOrTransaction } from "@trigger.dev/database";
10-
import { validateScheduleWindowAgainstCron } from "../scheduleWindow.server";
10+
import { validateScheduleWindowSyntax } from "../scheduleWindow.server";
1111

1212
type Schedule = {
1313
cron: string;
@@ -42,11 +42,7 @@ export class CheckScheduleService extends BaseService {
4242
}
4343
}
4444

45-
const windowValidation = validateScheduleWindowAgainstCron({
46-
window: schedule.window,
47-
cron: schedule.cron,
48-
timezone: schedule.timezone ?? "UTC",
49-
});
45+
const windowValidation = validateScheduleWindowSyntax(schedule.window);
5046
if (!windowValidation.valid) {
5147
throw new ServiceValidationError(windowValidation.message);
5248
}

‎apps/webapp/test/scheduleWindow.test.ts‎

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest";
22
import {
33
formatScheduleWindow,
44
normalizeScheduleWindow,
5-
validateScheduleWindowAgainstCron,
5+
validateScheduleWindowSyntax,
66
} from "~/v3/scheduleWindow.server";
77

88
describe("schedule window persistence", () => {
@@ -37,7 +37,7 @@ describe("schedule window persistence", () => {
3737
windowDurationSeconds: 86_400,
3838
windowPercentage: null,
3939
})
40-
).toBe("1d");
40+
).toBe("24h");
4141
expect(
4242
formatScheduleWindow({
4343
windowDurationSeconds: 7_200,
@@ -52,31 +52,14 @@ describe("schedule window persistence", () => {
5252
).toBe("30%");
5353
});
5454

55-
it("rejects invalid syntax through the authoritative timing parser", () => {
56-
expect(
57-
validateScheduleWindowAgainstCron({
58-
window: "30.5%",
59-
cron: "0 * * * *",
60-
timezone: "UTC",
61-
})
62-
).toMatchObject({ valid: false });
63-
});
64-
65-
it("rejects an absolute window longer than the next nominal interval", () => {
66-
expect(
67-
validateScheduleWindowAgainstCron({
68-
window: "30m",
69-
cron: "*/5 * * * *",
70-
timezone: "UTC",
71-
})
72-
).toMatchObject({ valid: false });
55+
it.each(["30.5%", "1d", "25h"])(
56+
"rejects invalid syntax through the authoritative timing parser: %s",
57+
(window) => {
58+
expect(validateScheduleWindowSyntax(window)).toMatchObject({ valid: false });
59+
}
60+
);
7361

74-
expect(
75-
validateScheduleWindowAgainstCron({
76-
window: "5m",
77-
cron: "*/5 * * * *",
78-
timezone: "UTC",
79-
})
80-
).toEqual({ valid: true });
62+
it("accepts an absolute window independently of the cron interval", () => {
63+
expect(validateScheduleWindowSyntax("30m")).toEqual({ valid: true });
8164
});
8265
});

‎apps/webapp/test/schedules-api.e2e.full.test.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,19 @@ describe("Schedules API windows", () => {
7777
});
7878
});
7979

80-
it("accepts zero duration and percentage windows", async () => {
80+
it("accepts zero windows and absolute windows longer than the cron interval", async () => {
8181
const server = getTestServer();
8282
const { apiKey, project, environment } = await seedTestEnvironment(server.prisma);
8383
await seedScheduledTask(server.prisma, project.id, environment.id);
8484

85-
for (const [index, window] of ["0m", "0h", "0d", "0%"].entries()) {
85+
const windows = [
86+
["0m", "0m"],
87+
["0h", "0m"],
88+
["0%", "0%"],
89+
["2h", "2h"],
90+
] as const;
91+
92+
for (const [index, [window, expectedWindow]] of windows.entries()) {
8693
const response = await server.webapp.fetch("/api/v1/schedules", {
8794
method: "POST",
8895
headers: authHeaders(apiKey),
@@ -95,9 +102,7 @@ describe("Schedules API windows", () => {
95102
});
96103

97104
expect(response.status).toBe(200);
98-
await expect(response.json()).resolves.toMatchObject({
99-
window: window === "0%" ? "0%" : "0m",
100-
});
105+
await expect(response.json()).resolves.toMatchObject({ window: expectedWindow });
101106
}
102107
});
103108

@@ -109,7 +114,8 @@ describe("Schedules API windows", () => {
109114
const invalidRequests = [
110115
{ window: 30, expectedStatus: 400 },
111116
{ window: "30.5%", expectedStatus: 422 },
112-
{ window: "2h", expectedStatus: 422 },
117+
{ window: "1d", expectedStatus: 422 },
118+
{ window: "25h", expectedStatus: 422 },
113119
];
114120

115121
for (const [index, { window, expectedStatus }] of invalidRequests.entries()) {

‎internal-packages/schedule-engine/src/engine/index.ts‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export class ScheduleEngine {
3636
private scheduleExecutionDuration: Histogram;
3737
private scheduleExecutionFailureCounter: Counter;
3838
private distributionOffsetHistogram: Histogram;
39+
private scheduleWindowCappedCounter: Counter;
3940
private devEnvironmentCheckCounter: Counter;
4041

4142
prisma: PrismaClient;
@@ -81,6 +82,10 @@ export class ScheduleEngine {
8182
}
8283
);
8384

85+
this.scheduleWindowCappedCounter = this.meter.createCounter("schedule_windows_capped_total", {
86+
description: "Total number of absolute schedule windows capped at the next nominal interval",
87+
});
88+
8489
this.devEnvironmentCheckCounter = this.meter.createCounter("dev_environment_checks_total", {
8590
description: "Total number of development environment connectivity checks",
8691
});
@@ -230,7 +235,8 @@ export class ScheduleEngine {
230235
effectiveRangeMs,
231236
windowMs,
232237
offsetMs: candidateDelayMs,
233-
rangeWasClamped,
238+
intervalMs,
239+
windowWasCappedToInterval,
234240
} = calculateEffectiveScheduleTime({
235241
nominalAt,
236242
nextNominalAt,
@@ -249,7 +255,18 @@ export class ScheduleEngine {
249255
span.setAttribute("applied_delay_ms", appliedDelayMs);
250256
span.setAttribute("schedule_window_ms", windowMs);
251257
span.setAttribute("effective_range_ms", effectiveRangeMs);
252-
span.setAttribute("schedule_range_was_clamped", rangeWasClamped);
258+
span.setAttribute("schedule_window_was_capped_to_interval", windowWasCappedToInterval);
259+
260+
if (windowWasCappedToInterval) {
261+
span.addEvent("schedule_window_capped_to_interval", {
262+
requested_window_ms: windowMs,
263+
nominal_interval_ms: intervalMs,
264+
});
265+
this.scheduleWindowCappedCounter.add(1, {
266+
environment_type: instance.environment.type,
267+
schedule_type: instance.taskSchedule.type,
268+
});
269+
}
253270

254271
const schedulingDelayMs = effectiveAt.getTime() - Date.now();
255272
span.setAttribute("scheduling_delay_ms", schedulingDelayMs);
@@ -265,7 +282,7 @@ export class ScheduleEngine {
265282
candidateDelayMs,
266283
appliedDelayMs,
267284
effectiveRangeMs,
268-
rangeWasClamped,
285+
windowWasCappedToInterval,
269286
schedulingDelayMs,
270287
generatorExpression: instance.taskSchedule.generatorExpression,
271288
timezone: instance.taskSchedule.timezone,

‎internal-packages/schedule-engine/src/engine/scheduleTiming.test.ts‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
MAX_ABSOLUTE_SCHEDULE_WINDOW_SECONDS,
23
MAX_SCHEDULE_PHASE,
34
MINIMUM_SCHEDULE_RANGE_MS,
45
SCHEDULE_PHASE_DENOMINATOR,
@@ -7,17 +8,15 @@ import {
78
parseScheduleWindow,
89
resolveScheduleWindowMs,
910
validateScheduleWindow,
10-
validateScheduleWindowForInterval,
1111
} from "./scheduleTiming.js";
1212

1313
describe("parseScheduleWindow", () => {
1414
it.each([
1515
["30m", { type: "duration", durationSeconds: 1_800 }],
1616
["2h", { type: "duration", durationSeconds: 7_200 }],
17-
["1d", { type: "duration", durationSeconds: 86_400 }],
17+
["24h", { type: "duration", durationSeconds: 86_400 }],
1818
["0m", { type: "duration", durationSeconds: 0 }],
1919
["0h", { type: "duration", durationSeconds: 0 }],
20-
["0d", { type: "duration", durationSeconds: 0 }],
2120
["0%", { type: "percentage", percentage: 0 }],
2221
["12%", { type: "percentage", percentage: 12 }],
2322
["100%", { type: "percentage", percentage: 100 }],
@@ -30,6 +29,10 @@ describe("parseScheduleWindow", () => {
3029
"00m",
3130
"01m",
3231
"1.5h",
32+
"0d",
33+
"1d",
34+
"25h",
35+
"1441m",
3336
"30s",
3437
"0.01%",
3538
"1.0%",
@@ -44,8 +47,13 @@ describe("parseScheduleWindow", () => {
4447
expect(() => parseScheduleWindow(input)).toThrow();
4548
});
4649

47-
it("rejects durations that cannot be persisted as a Postgres Int", () => {
48-
expect(() => parseScheduleWindow("24856d")).toThrow("duration is too large");
50+
it("rejects normalized durations over 24 hours", () => {
51+
expect(() =>
52+
validateScheduleWindow({
53+
type: "duration",
54+
durationSeconds: MAX_ABSOLUTE_SCHEDULE_WINDOW_SECONDS + 1,
55+
})
56+
).toThrow("up to 24 hours");
4957
});
5058
});
5159

@@ -58,18 +66,6 @@ describe("schedule window validation", () => {
5866
expect(() => validateScheduleWindow({ type: "duration", durationSeconds: 0 })).not.toThrow();
5967
});
6068

61-
it("allows an absolute window equal to the nominal interval", () => {
62-
expect(() =>
63-
validateScheduleWindowForInterval({ type: "duration", durationSeconds: 300 }, 5 * 60_000)
64-
).not.toThrow();
65-
});
66-
67-
it("rejects an absolute window larger than the nominal interval", () => {
68-
expect(() =>
69-
validateScheduleWindowForInterval({ type: "duration", durationSeconds: 1_800 }, 5 * 60_000)
70-
).toThrow("cannot exceed the interval");
71-
});
72-
7369
it.each([
7470
{ type: "duration", durationSeconds: -1 },
7571
{ type: "duration", durationSeconds: 1.5 },
@@ -111,7 +107,7 @@ describe("calculateEffectiveScheduleTime", () => {
111107
windowMs: 0,
112108
effectiveRangeMs: MINIMUM_SCHEDULE_RANGE_MS,
113109
offsetMs: 30_000,
114-
rangeWasClamped: false,
110+
windowWasCappedToInterval: false,
115111
});
116112
});
117113

@@ -189,7 +185,7 @@ describe("calculateEffectiveScheduleTime", () => {
189185
expect(timing.effectiveAt).toEqual(new Date("2027-01-01T00:30:00.000Z"));
190186
});
191187

192-
it("defensively clamps an invalid range to the next nominal tick", () => {
188+
it("caps an absolute window at the interval to the next nominal tick", () => {
193189
const timing = calculateEffectiveScheduleTime({
194190
nominalAt,
195191
nextNominalAt: new Date("2026-08-10T10:05:00.000Z"),
@@ -199,7 +195,7 @@ describe("calculateEffectiveScheduleTime", () => {
199195

200196
expect(timing.windowMs).toBe(1_800_000);
201197
expect(timing.effectiveRangeMs).toBe(300_000);
202-
expect(timing.rangeWasClamped).toBe(true);
198+
expect(timing.windowWasCappedToInterval).toBe(true);
203199
expect(timing.effectiveAt).toEqual(new Date("2026-08-10T10:02:30.000Z"));
204200
});
205201

0 commit comments

Comments
 (0)