Skip to content

Commit 61d77ea

Browse files
committed
fix(webapp): reject debounce windows that cannot collapse anything
Validation only looked at a caller-supplied maxDelay, so a deployment that configures a server ceiling kept the original silent failure: a delay at or above it accepted the trigger and then created a run per trigger. The check now runs against the effective ceiling, whichever of the two applies, and a delay given as a date is rejected as well since the value is re-applied on every push and an absolute date can never work.
1 parent 13da597 commit 61d77ea

4 files changed

Lines changed: 100 additions & 23 deletions

File tree

.changeset/debounce-max-duration.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Debouncing with a `delay` longer than an hour now works. A hidden server-side li
66

77
That limit is gone. A debounce key with no `maxDelay` now keeps pushing its run back for as long as triggers keep arriving, which means it never executes while they do. Set `maxDelay` when the work has to happen eventually, and keep `delay` well below it, since the room available to push is the gap between the two.
88

9-
Triggers that set a `maxDelay` no longer than their `delay` are now rejected, as are unparseable `maxDelay` values. Both previously went through and left debouncing doing nothing.
9+
Triggers we know cannot debounce are now rejected instead of quietly doing nothing: a `maxDelay` no longer than the `delay`, an unparseable `maxDelay`, and a `delay` given as a date rather than a duration. Self-hosters who configure a maximum debounce duration get the same treatment for a `delay` at or above it.
1010

1111
```ts
1212
await myTask.trigger(payload, {

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

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type { Tracer } from "@opentelemetry/api";
77
import { tryCatch } from "@trigger.dev/core/utils";
88
import {
99
type TriggerTaskRequestBody,
10+
formatDurationMilliseconds,
1011
RunAnnotations,
1112
TaskRunError,
1213
taskRunErrorEnhancer,
@@ -90,6 +91,7 @@ export class RunEngineTriggerTaskService {
9091
private readonly traceEventConcern: TraceEventConcern;
9192
private readonly triggerRacepointSystem: TriggerRacepointSystem;
9293
private readonly metadataMaximumSize: number;
94+
private readonly maximumDebounceDurationMs: number | undefined;
9395
// Mollifier hooks are DI'd so tests can drive the call-site's mollify branch
9496
// deterministically (stub the gate to return mollify, inject a real or fake
9597
// buffer, force the global-enabled predicate to true so the call site
@@ -109,6 +111,7 @@ export class RunEngineTriggerTaskService {
109111
traceEventConcern: TraceEventConcern;
110112
tracer: Tracer;
111113
metadataMaximumSize: number;
114+
maximumDebounceDurationMs?: number;
112115
triggerRacepointSystem?: TriggerRacepointSystem;
113116
evaluateGate?: MollifierEvaluateGate;
114117
getMollifierBuffer?: MollifierGetBuffer;
@@ -123,6 +126,8 @@ export class RunEngineTriggerTaskService {
123126
this.tracer = opts.tracer;
124127
this.traceEventConcern = opts.traceEventConcern;
125128
this.metadataMaximumSize = opts.metadataMaximumSize;
129+
this.maximumDebounceDurationMs =
130+
opts.maximumDebounceDurationMs ?? env.RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS;
126131
this.triggerRacepointSystem = opts.triggerRacepointSystem ?? new NoopTriggerRacepointSystem();
127132
this.evaluateGate = opts.evaluateGate ?? defaultEvaluateGate;
128133
this.getMollifierBuffer = opts.getMollifierBuffer ?? defaultGetMollifierBuffer;
@@ -131,36 +136,60 @@ export class RunEngineTriggerTaskService {
131136
}
132137

133138
/**
134-
* A debounced run is only pushed back while its new execution time stays inside `maxDelay`,
135-
* so the room available to push is `maxDelay` minus `delay`. When a caller sets both and
136-
* leaves no room, the debounce key silently does nothing and every trigger creates its own
137-
* run, which is worse than being told. Only an explicit `maxDelay` is checked; with no
138-
* `maxDelay` there is no ceiling to conflict with.
139+
* A debounced run is only pushed back while its new execution time stays inside the effective
140+
* ceiling, which is the trigger's own `maxDelay` or, failing that, whatever ceiling the server
141+
* is configured with. The room available to push is that ceiling minus `delay`, so a `delay`
142+
* at or above it leaves none: the debounce key does nothing and every trigger creates its own
143+
* run. Rejecting is better than accepting a trigger we know cannot debounce.
144+
*
145+
* With no `maxDelay` and no server ceiling there is nothing to conflict with, which is the
146+
* default.
139147
*/
140-
#validateDebounceMaxDelay(
148+
#validateDebounceWindow(
141149
debounce: NonNullable<NonNullable<TriggerTaskRequestBody["options"]>["debounce"]>
142150
) {
143-
if (debounce.maxDelay === undefined) {
144-
return;
145-
}
146-
147-
const maxDelayMs = parseNaturalLanguageDurationInMs(debounce.maxDelay);
151+
const delayMs = parseNaturalLanguageDurationInMs(debounce.delay);
148152

149-
if (maxDelayMs === undefined) {
153+
if (delayMs === undefined) {
150154
throw new ServiceValidationError(
151-
`Invalid debounce maxDelay: ${debounce.maxDelay}. ` +
152-
`Supported formats: {number}s, {number}m, {number}h or {number}hr, {number}d, {number}w, ` +
153-
`optionally combined (for example "2h30m").`
155+
`Invalid debounce delay: ${debounce.delay}. debounce.delay must be a duration, not a ` +
156+
`date, because it is re-applied every time the run is pushed back. Supported formats: ` +
157+
`{number}s, {number}m, {number}h or {number}hr, {number}d, {number}w, optionally ` +
158+
`combined (for example "2h30m").`
154159
);
155160
}
156161

157-
const delayMs = parseNaturalLanguageDurationInMs(debounce.delay);
162+
if (debounce.maxDelay !== undefined) {
163+
const maxDelayMs = parseNaturalLanguageDurationInMs(debounce.maxDelay);
164+
165+
if (maxDelayMs === undefined) {
166+
throw new ServiceValidationError(
167+
`Invalid debounce maxDelay: ${debounce.maxDelay}. ` +
168+
`Supported formats: {number}s, {number}m, {number}h or {number}hr, {number}d, ` +
169+
`{number}w, optionally combined (for example "2h30m").`
170+
);
171+
}
172+
173+
if (maxDelayMs <= delayMs) {
174+
throw new ServiceValidationError(
175+
`debounce.maxDelay (${debounce.maxDelay}) must be longer than debounce.delay ` +
176+
`(${debounce.delay}). A debounced run is only pushed back while it stays inside ` +
177+
`maxDelay, so with these values every trigger would create its own run.`
178+
);
179+
}
180+
181+
return;
182+
}
183+
184+
const serverCeilingMs = this.maximumDebounceDurationMs;
158185

159-
if (delayMs !== undefined && maxDelayMs <= delayMs) {
186+
if (serverCeilingMs !== undefined && delayMs >= serverCeilingMs) {
160187
throw new ServiceValidationError(
161-
`debounce.maxDelay (${debounce.maxDelay}) must be longer than debounce.delay (${debounce.delay}). ` +
162-
`A debounced run is only pushed back while it stays inside maxDelay, so with these values ` +
163-
`every trigger would create its own run.`
188+
`debounce.delay (${debounce.delay}) is at or above this server's maximum debounce ` +
189+
`duration of ${formatDurationMilliseconds(serverCeilingMs, { style: "short" })}. A ` +
190+
`debounced run is only pushed back while it stays inside that window, so with this ` +
191+
`delay every trigger would create its own run. Either shorten the delay, or set ` +
192+
`debounce.maxDelay above ${debounce.delay}.`
164193
);
165194
}
166195
}
@@ -312,7 +341,7 @@ export class RunEngineTriggerTaskService {
312341
);
313342
}
314343

315-
this.#validateDebounceMaxDelay(body.options.debounce);
344+
this.#validateDebounceWindow(body.options.debounce);
316345
}
317346

318347
const parentRun = body.options?.parentRunId

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,54 @@ describe("RunEngineTriggerTaskService", () => {
556556

557557
const noMaxDelay = await triggerWithDebounce({ key: "no-max-delay", delay: "12h" });
558558
expect(noMaxDelay?.run.friendlyId).toBeDefined();
559+
560+
await expect(
561+
triggerWithDebounce({ key: "date-delay", delay: "2027-01-01T00:00:00.000Z" })
562+
).rejects.toThrow(/must be a duration, not a date/);
563+
564+
const withServerCeiling = new RunEngineTriggerTaskService({
565+
engine,
566+
prisma,
567+
payloadProcessor: new MockPayloadProcessor(),
568+
queueConcern: queuesManager,
569+
idempotencyKeyConcern,
570+
validator: new MockTriggerTaskValidator(),
571+
traceEventConcern: new MockTraceEventConcern(),
572+
tracer: trace.getTracer("test", "0.0.0"),
573+
metadataMaximumSize: 1024 * 1024 * 1,
574+
maximumDebounceDurationMs: 60 * 60 * 1000,
575+
});
576+
577+
await expect(
578+
withServerCeiling.call({
579+
taskId: taskIdentifier,
580+
environment: authenticatedEnvironment,
581+
body: {
582+
payload: { test: "test" },
583+
options: { debounce: { key: "over-server-ceiling", delay: "12h" } },
584+
},
585+
})
586+
).rejects.toThrow(/at or above this server's maximum debounce duration of 1h/);
587+
588+
const underServerCeiling = await withServerCeiling.call({
589+
taskId: taskIdentifier,
590+
environment: authenticatedEnvironment,
591+
body: {
592+
payload: { test: "test" },
593+
options: { debounce: { key: "under-server-ceiling", delay: "10s" } },
594+
},
595+
});
596+
expect(underServerCeiling?.run.friendlyId).toBeDefined();
597+
598+
const overCeilingWithMaxDelay = await withServerCeiling.call({
599+
taskId: taskIdentifier,
600+
environment: authenticatedEnvironment,
601+
body: {
602+
payload: { test: "test" },
603+
options: { debounce: { key: "override", delay: "12h", maxDelay: "24h" } },
604+
},
605+
});
606+
expect(overCeilingWithMaxDelay?.run.friendlyId).toBeDefined();
559607
}
560608
);
561609
});

docs/triggering.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -928,7 +928,7 @@ Consider `delay: "5s"` and `maxDelay: "30s"` with triggers arriving every 2 seco
928928

929929
Without `maxDelay`, continuous triggers would prevent the run from ever executing. With `maxDelay: "30s"`, execution is guaranteed within 30 seconds of the first trigger.
930930

931-
Keep `delay` well below `maxDelay`. A run is only pushed back while its new execution time stays inside `maxDelay`, so the room you have to push is `maxDelay` minus `delay`. Setting them equal, or setting `delay` higher, leaves no room at all: every trigger creates its own run and the debounce key has no effect.
931+
Keep `delay` well below `maxDelay`. A run is only pushed back while its new execution time stays inside `maxDelay`, so the room you have to push is `maxDelay` minus `delay`. Setting them equal, or setting `delay` higher, leaves no room at all, and the trigger is rejected rather than accepted as a debounce that could never collapse anything.
932932

933933
<Note>
934934
The `maxDelay` value is evaluated from each trigger call, not stored with the original run. This

0 commit comments

Comments
 (0)