Skip to content

Commit 20f3a88

Browse files
ericallamericallam
committed
better handle invalid debounce delay options
1 parent d78877b commit 20f3a88

File tree

2 files changed

+253
-3
lines changed

2 files changed

+253
-3
lines changed

‎apps/webapp/app/runEngine/services/triggerTask.server.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,25 @@ export class RunEngineTriggerTaskService {
169169
}
170170

171171
// Validate debounce options
172-
if (body.options?.debounce && !delayUntil) {
173-
throw new ServiceValidationError(
174-
`Debounce requires a valid delay duration. Provided: ${body.options.debounce.delay}`
172+
if (body.options?.debounce) {
173+
if (!delayUntil) {
174+
throw new ServiceValidationError(
175+
`Debounce requires a valid delay duration. Provided: ${body.options.debounce.delay}`
176+
);
177+
}
178+
179+
// Always validate debounce.delay separately since it's used for rescheduling
180+
// This catches the case where options.delay is valid but debounce.delay is invalid
181+
const [debounceDelayError, debounceDelayUntil] = await tryCatch(
182+
parseDelay(body.options.debounce.delay)
175183
);
184+
185+
if (debounceDelayError || !debounceDelayUntil) {
186+
throw new ServiceValidationError(
187+
`Invalid debounce delay: ${body.options.debounce.delay}. ` +
188+
`Supported formats: {number}s, {number}m, {number}h, {number}d, {number}w`
189+
);
190+
}
176191
}
177192

178193
const ttl =

‎apps/webapp/test/engine/triggerTask.test.ts‎

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -937,4 +937,239 @@ describe("RunEngineTriggerTaskService", () => {
937937
await engine.quit();
938938
}
939939
);
940+
941+
containerTest(
942+
"should reject invalid debounce.delay when no explicit delay is provided",
943+
async ({ prisma, redisOptions }) => {
944+
const engine = new RunEngine({
945+
prisma,
946+
worker: {
947+
redis: redisOptions,
948+
workers: 1,
949+
tasksPerWorker: 10,
950+
pollIntervalMs: 100,
951+
},
952+
queue: {
953+
redis: redisOptions,
954+
},
955+
runLock: {
956+
redis: redisOptions,
957+
},
958+
machines: {
959+
defaultMachine: "small-1x",
960+
machines: {
961+
"small-1x": {
962+
name: "small-1x" as const,
963+
cpu: 0.5,
964+
memory: 0.5,
965+
centsPerMs: 0.0001,
966+
},
967+
},
968+
baseCostInCents: 0.0005,
969+
},
970+
tracer: trace.getTracer("test", "0.0.0"),
971+
});
972+
973+
const authenticatedEnvironment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
974+
const taskIdentifier = "test-task";
975+
976+
await setupBackgroundWorker(engine, authenticatedEnvironment, taskIdentifier);
977+
978+
const queuesManager = new DefaultQueueManager(prisma, engine);
979+
const idempotencyKeyConcern = new IdempotencyKeyConcern(
980+
prisma,
981+
engine,
982+
new MockTraceEventConcern()
983+
);
984+
985+
const triggerTaskService = new RunEngineTriggerTaskService({
986+
engine,
987+
prisma,
988+
payloadProcessor: new MockPayloadProcessor(),
989+
queueConcern: queuesManager,
990+
idempotencyKeyConcern,
991+
validator: new MockTriggerTaskValidator(),
992+
traceEventConcern: new MockTraceEventConcern(),
993+
tracer: trace.getTracer("test", "0.0.0"),
994+
metadataMaximumSize: 1024 * 1024 * 1,
995+
});
996+
997+
// Invalid debounce.delay format (ms not supported)
998+
await expect(
999+
triggerTaskService.call({
1000+
taskId: taskIdentifier,
1001+
environment: authenticatedEnvironment,
1002+
body: {
1003+
payload: { test: "test" },
1004+
options: {
1005+
debounce: {
1006+
key: "test-key",
1007+
delay: "300ms", // Invalid - ms not supported
1008+
},
1009+
},
1010+
},
1011+
})
1012+
).rejects.toThrow("Debounce requires a valid delay duration");
1013+
1014+
await engine.quit();
1015+
}
1016+
);
1017+
1018+
containerTest(
1019+
"should reject invalid debounce.delay even when explicit delay is valid",
1020+
async ({ prisma, redisOptions }) => {
1021+
const engine = new RunEngine({
1022+
prisma,
1023+
worker: {
1024+
redis: redisOptions,
1025+
workers: 1,
1026+
tasksPerWorker: 10,
1027+
pollIntervalMs: 100,
1028+
},
1029+
queue: {
1030+
redis: redisOptions,
1031+
},
1032+
runLock: {
1033+
redis: redisOptions,
1034+
},
1035+
machines: {
1036+
defaultMachine: "small-1x",
1037+
machines: {
1038+
"small-1x": {
1039+
name: "small-1x" as const,
1040+
cpu: 0.5,
1041+
memory: 0.5,
1042+
centsPerMs: 0.0001,
1043+
},
1044+
},
1045+
baseCostInCents: 0.0005,
1046+
},
1047+
tracer: trace.getTracer("test", "0.0.0"),
1048+
});
1049+
1050+
const authenticatedEnvironment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
1051+
const taskIdentifier = "test-task";
1052+
1053+
await setupBackgroundWorker(engine, authenticatedEnvironment, taskIdentifier);
1054+
1055+
const queuesManager = new DefaultQueueManager(prisma, engine);
1056+
const idempotencyKeyConcern = new IdempotencyKeyConcern(
1057+
prisma,
1058+
engine,
1059+
new MockTraceEventConcern()
1060+
);
1061+
1062+
const triggerTaskService = new RunEngineTriggerTaskService({
1063+
engine,
1064+
prisma,
1065+
payloadProcessor: new MockPayloadProcessor(),
1066+
queueConcern: queuesManager,
1067+
idempotencyKeyConcern,
1068+
validator: new MockTriggerTaskValidator(),
1069+
traceEventConcern: new MockTraceEventConcern(),
1070+
tracer: trace.getTracer("test", "0.0.0"),
1071+
metadataMaximumSize: 1024 * 1024 * 1,
1072+
});
1073+
1074+
// Valid explicit delay but invalid debounce.delay
1075+
// This is the bug case: the explicit delay passes validation,
1076+
// but debounce.delay would fail later when rescheduling
1077+
await expect(
1078+
triggerTaskService.call({
1079+
taskId: taskIdentifier,
1080+
environment: authenticatedEnvironment,
1081+
body: {
1082+
payload: { test: "test" },
1083+
options: {
1084+
delay: "5m", // Valid explicit delay
1085+
debounce: {
1086+
key: "test-key",
1087+
delay: "invalid-delay", // Invalid debounce delay
1088+
},
1089+
},
1090+
},
1091+
})
1092+
).rejects.toThrow("Invalid debounce delay");
1093+
1094+
await engine.quit();
1095+
}
1096+
);
1097+
1098+
containerTest(
1099+
"should accept valid debounce.delay formats",
1100+
async ({ prisma, redisOptions }) => {
1101+
const engine = new RunEngine({
1102+
prisma,
1103+
worker: {
1104+
redis: redisOptions,
1105+
workers: 1,
1106+
tasksPerWorker: 10,
1107+
pollIntervalMs: 100,
1108+
},
1109+
queue: {
1110+
redis: redisOptions,
1111+
},
1112+
runLock: {
1113+
redis: redisOptions,
1114+
},
1115+
machines: {
1116+
defaultMachine: "small-1x",
1117+
machines: {
1118+
"small-1x": {
1119+
name: "small-1x" as const,
1120+
cpu: 0.5,
1121+
memory: 0.5,
1122+
centsPerMs: 0.0001,
1123+
},
1124+
},
1125+
baseCostInCents: 0.0005,
1126+
},
1127+
tracer: trace.getTracer("test", "0.0.0"),
1128+
});
1129+
1130+
const authenticatedEnvironment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
1131+
const taskIdentifier = "test-task";
1132+
1133+
await setupBackgroundWorker(engine, authenticatedEnvironment, taskIdentifier);
1134+
1135+
const queuesManager = new DefaultQueueManager(prisma, engine);
1136+
const idempotencyKeyConcern = new IdempotencyKeyConcern(
1137+
prisma,
1138+
engine,
1139+
new MockTraceEventConcern()
1140+
);
1141+
1142+
const triggerTaskService = new RunEngineTriggerTaskService({
1143+
engine,
1144+
prisma,
1145+
payloadProcessor: new MockPayloadProcessor(),
1146+
queueConcern: queuesManager,
1147+
idempotencyKeyConcern,
1148+
validator: new MockTriggerTaskValidator(),
1149+
traceEventConcern: new MockTraceEventConcern(),
1150+
tracer: trace.getTracer("test", "0.0.0"),
1151+
metadataMaximumSize: 1024 * 1024 * 1,
1152+
});
1153+
1154+
// Valid debounce.delay format
1155+
const result = await triggerTaskService.call({
1156+
taskId: taskIdentifier,
1157+
environment: authenticatedEnvironment,
1158+
body: {
1159+
payload: { test: "test" },
1160+
options: {
1161+
debounce: {
1162+
key: "test-key",
1163+
delay: "5s", // Valid format
1164+
},
1165+
},
1166+
},
1167+
});
1168+
1169+
expect(result).toBeDefined();
1170+
expect(result?.run.friendlyId).toBeDefined();
1171+
1172+
await engine.quit();
1173+
}
1174+
);
9401175
});

0 commit comments

Comments
 (0)