Skip to content

Commit c3f1c11

Browse files
claude[bot]Trigger.dev RepoOps
authored andcommitted
fix(webapp): make request idempotency work with scoped keys
Request idempotency was doing nothing. The key a client sends is hashed together with its environment and task scope before it is looked up, but the idempotency cache still required the raw UUID format and rejected the hashed value, so every trigger, batch trigger and create-batch request skipped deduplication. A request the server had already accepted, whose response the client never received, would therefore create a second run or batch when the SDK retried it. The cache now accepts the derived key, so a retried request returns the original run or batch. The client-supplied `x-trigger-request-idempotency-key` header is still validated as a v4 UUID, now at the point where it is read; a header of any other shape is ignored as before, so the request itself still succeeds. Keys stay scoped to a single environment, task set and request type. Mono-RevId: 85b6e31f5bdb722d99db28f66a2d5b533a0f3ce4
1 parent 0db5de5 commit c3f1c11

8 files changed

Lines changed: 142 additions & 30 deletions
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Retried trigger and batch trigger requests are deduplicated again: when the SDK automatically retries a request that the server had in fact already accepted, you get the original run or batch back instead of a duplicate one.

‎apps/webapp/app/routes/api.v1.tasks.$taskId.trigger.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
handleRequestIdempotency,
2121
saveRequestIdempotency,
2222
} from "~/utils/requestIdempotency.server";
23-
import { scopeRequestIdempotencyKey } from "~/utils/requestIdempotencyKey";
23+
import { scopeRequestIdempotencyHeader } from "~/utils/requestIdempotencyKey";
2424
import { canWriteParentRun } from "~/utils/parentRunAuthorization.server";
2525
import { sanitizeTriggerSource } from "~/utils/triggerSource";
2626
import { runStore } from "~/v3/runStore.server";
@@ -89,7 +89,7 @@ const { action, loader } = createActionApiRoute(
8989
return json({ error: "Unauthorized" }, { status: 403 });
9090
}
9191

92-
const scopedIdempotencyKey = scopeRequestIdempotencyKey(requestIdempotencyKey, [
92+
const scopedIdempotencyKey = scopeRequestIdempotencyHeader(requestIdempotencyKey, [
9393
authentication.environment.id,
9494
params.taskId,
9595
]);

‎apps/webapp/app/routes/api.v2.tasks.batch.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
handleRequestIdempotency,
1313
saveRequestIdempotency,
1414
} from "~/utils/requestIdempotency.server";
15-
import { scopeRequestIdempotencyKey } from "~/utils/requestIdempotencyKey";
15+
import { scopeRequestIdempotencyHeader } from "~/utils/requestIdempotencyKey";
1616
import { canWriteParentRun } from "~/utils/parentRunAuthorization.server";
1717
import { ServiceValidationError } from "~/v3/services/baseService.server";
1818
import { BatchProcessingStrategy } from "~/v3/services/batchTriggerV3.server";
@@ -100,7 +100,7 @@ const { action, loader } = createActionApiRoute(
100100
requestIdempotencyKey,
101101
});
102102

103-
const scopedIdempotencyKey = scopeRequestIdempotencyKey(requestIdempotencyKey, [
103+
const scopedIdempotencyKey = scopeRequestIdempotencyHeader(requestIdempotencyKey, [
104104
authentication.environment.id,
105105
...Array.from(new Set(body.items.map((item) => item.task))).sort(),
106106
]);

‎apps/webapp/app/services/requestIdempotency.server.ts‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,20 @@ import { createCache, DefaultStatefulContext, Namespace } from "@unkey/cache";
55
import { createLRUMemoryStore } from "@internal/cache";
66
import { RedisCacheStore } from "./unkey/redisCacheStore.server";
77
import type { RedisWithClusterOptions } from "~/redis.server";
8-
import { validate as uuidValidate, version as uuidVersion } from "uuid";
98
import { startActiveSpan } from "~/v3/tracer.server";
109

1110
export type RequestIdempotencyServiceOptions<TTypes extends string> = {
1211
types: TTypes[];
13-
redis: RedisWithClusterOptions;
12+
redis?: RedisWithClusterOptions;
1413
logger?: Logger;
1514
logLevel?: LogLevel;
1615
ttlInMs?: number;
1716
};
1817

1918
const DEFAULT_TTL_IN_MS = 60_000 * 60 * 24;
2019

20+
const SCOPED_REQUEST_ID_REGEX = /^[0-9a-f]{64}$/;
21+
2122
type RequestIdempotencyCacheEntry = {
2223
id: string;
2324
};
@@ -30,24 +31,24 @@ export class RequestIdempotencyService<TTypes extends string> {
3031
this.logger =
3132
options.logger ?? new Logger("RequestIdempotencyService", options.logLevel ?? "info");
3233

33-
const keyPrefix = options.redis.keyPrefix
34-
? `request-idempotency:${options.redis.keyPrefix}`
35-
: "request-idempotency:";
36-
3734
const ctx = new DefaultStatefulContext();
3835
const memory = createLRUMemoryStore(1000);
39-
const redisCacheStore = new RedisCacheStore({
40-
name: "request-idempotency",
41-
connection: {
42-
keyPrefix: keyPrefix,
43-
...options.redis,
44-
},
45-
});
36+
const redisCacheStore = options.redis
37+
? new RedisCacheStore({
38+
name: "request-idempotency",
39+
connection: {
40+
keyPrefix: options.redis.keyPrefix
41+
? `request-idempotency:${options.redis.keyPrefix}`
42+
: "request-idempotency:",
43+
...options.redis,
44+
},
45+
})
46+
: undefined;
4647

4748
// This cache holds the rate limit configuration for each org, so we don't have to fetch it every request
4849
const cache = createCache({
4950
requests: new Namespace<RequestIdempotencyCacheEntry>(ctx, {
50-
stores: [memory, redisCacheStore],
51+
stores: redisCacheStore ? [memory, redisCacheStore] : [memory],
5152
fresh: options.ttlInMs ?? DEFAULT_TTL_IN_MS,
5253
stale: options.ttlInMs ?? DEFAULT_TTL_IN_MS,
5354
}),
@@ -115,12 +116,8 @@ export class RequestIdempotencyService<TTypes extends string> {
115116
return result;
116117
}
117118

118-
// The requestIdempotencyKey should be a valid UUID
119+
// Keys are server-derived by `scopeRequestIdempotencyKey()`, so only the shape needs checking.
119120
#validateRequestId(requestIdempotencyKey: string): boolean {
120-
return isValidV4UUID(requestIdempotencyKey);
121+
return SCOPED_REQUEST_ID_REGEX.test(requestIdempotencyKey);
121122
}
122123
}
123-
124-
function isValidV4UUID(uuid: string): boolean {
125-
return uuidValidate(uuid) && uuidVersion(uuid) === 4;
126-
}

‎apps/webapp/app/utils/requestIdempotency.test.ts‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
import { randomUUID } from "node:crypto";
12
import { describe, expect, it } from "vitest";
2-
import { scopeRequestIdempotencyKey } from "./requestIdempotencyKey";
3+
import { scopeRequestIdempotencyHeader, scopeRequestIdempotencyKey } from "./requestIdempotencyKey";
34

45
describe("scopeRequestIdempotencyKey", () => {
56
it("keeps retries stable within the same environment and task scope", () => {
@@ -15,8 +16,37 @@ describe("scopeRequestIdempotencyKey", () => {
1516
expect(scopeRequestIdempotencyKey("request-1", ["env-2", "task-a"])).not.toBe(original);
1617
});
1718

18-
it("preserves missing request keys", () => {
19+
it("skips missing request keys", () => {
1920
expect(scopeRequestIdempotencyKey(undefined, ["env-1", "task-a"])).toBeUndefined();
20-
expect(scopeRequestIdempotencyKey(null, ["env-1", "task-a"])).toBeNull();
21+
expect(scopeRequestIdempotencyKey(null, ["env-1", "task-a"])).toBeUndefined();
22+
});
23+
});
24+
25+
describe("scopeRequestIdempotencyHeader", () => {
26+
it("scopes a v4 UUID header", () => {
27+
const requestIdempotencyKey = randomUUID();
28+
29+
expect(scopeRequestIdempotencyHeader(requestIdempotencyKey, ["env-1", "task-a"])).toBe(
30+
scopeRequestIdempotencyKey(requestIdempotencyKey, ["env-1", "task-a"])
31+
);
32+
});
33+
34+
it("ignores a header that is not a v4 UUID, instead of failing the request", () => {
35+
for (const requestIdempotencyKey of [
36+
"not-a-uuid",
37+
"",
38+
"00000000-0000-0000-0000-000000000000",
39+
"1b4e28ba-2fa1-11d2-883f-0016d3cca427", // v1
40+
`${randomUUID()} `,
41+
]) {
42+
expect(
43+
scopeRequestIdempotencyHeader(requestIdempotencyKey, ["env-1", "task-a"])
44+
).toBeUndefined();
45+
}
46+
});
47+
48+
it("skips missing headers", () => {
49+
expect(scopeRequestIdempotencyHeader(undefined, ["env-1", "task-a"])).toBeUndefined();
50+
expect(scopeRequestIdempotencyHeader(null, ["env-1", "task-a"])).toBeUndefined();
2151
});
2252
});
Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,26 @@
11
import { createHash } from "node:crypto";
2+
import { validate as uuidValidate, version as uuidVersion } from "uuid";
23

34
export function scopeRequestIdempotencyKey(
45
requestIdempotencyKey: string | null | undefined,
56
scope: readonly string[]
6-
): string | null | undefined {
7-
if (!requestIdempotencyKey) return requestIdempotencyKey;
7+
): string | undefined {
8+
if (!requestIdempotencyKey) return undefined;
89

910
return createHash("sha256")
1011
.update(JSON.stringify([requestIdempotencyKey, ...scope]))
1112
.digest("hex");
1213
}
14+
15+
export function scopeRequestIdempotencyHeader(
16+
requestIdempotencyKey: string | null | undefined,
17+
scope: readonly string[]
18+
): string | undefined {
19+
if (!requestIdempotencyKey || !isValidV4UUID(requestIdempotencyKey)) return undefined;
20+
21+
return scopeRequestIdempotencyKey(requestIdempotencyKey, scope);
22+
}
23+
24+
function isValidV4UUID(uuid: string): boolean {
25+
return uuidValidate(uuid) && uuidVersion(uuid) === 4;
26+
}
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { randomUUID } from "node:crypto";
2+
import { describe, expect, it } from "vitest";
3+
import { RequestIdempotencyService } from "~/services/requestIdempotency.server";
4+
import {
5+
scopeRequestIdempotencyHeader,
6+
scopeRequestIdempotencyKey,
7+
} from "~/utils/requestIdempotencyKey";
8+
9+
function createService() {
10+
return new RequestIdempotencyService({ types: ["trigger", "batch-trigger"] });
11+
}
12+
13+
describe("RequestIdempotencyService", () => {
14+
it("returns the saved entry for a scoped key", async () => {
15+
const service = createService();
16+
const scopedKey = scopeRequestIdempotencyHeader(randomUUID(), ["env-1", "task-a"]);
17+
18+
expect(scopedKey).toBeDefined();
19+
20+
await service.saveRequest("trigger", scopedKey!, { id: "run_1234" });
21+
22+
await expect(service.checkRequest("trigger", scopedKey!)).resolves.toEqual({ id: "run_1234" });
23+
});
24+
25+
it("keeps entries isolated by environment, task and request type", async () => {
26+
const service = createService();
27+
const requestIdempotencyKey = randomUUID();
28+
const scopedKey = scopeRequestIdempotencyHeader(requestIdempotencyKey, ["env-1", "task-a"]);
29+
30+
await service.saveRequest("trigger", scopedKey!, { id: "run_1234" });
31+
32+
const otherEnvironment = scopeRequestIdempotencyHeader(requestIdempotencyKey, [
33+
"env-2",
34+
"task-a",
35+
]);
36+
const otherTask = scopeRequestIdempotencyHeader(requestIdempotencyKey, ["env-1", "task-b"]);
37+
38+
await expect(service.checkRequest("trigger", otherEnvironment!)).resolves.toBeUndefined();
39+
await expect(service.checkRequest("trigger", otherTask!)).resolves.toBeUndefined();
40+
await expect(service.checkRequest("batch-trigger", scopedKey!)).resolves.toBeUndefined();
41+
});
42+
43+
it("accepts a scoped body key, not just a scoped header", async () => {
44+
const service = createService();
45+
const scopedKey = scopeRequestIdempotencyKey("my-batch-key", ["env-1", "task-a"]);
46+
47+
await service.saveRequest("batch-trigger", scopedKey!, { id: "batch_1234" });
48+
49+
await expect(service.checkRequest("batch-trigger", scopedKey!)).resolves.toEqual({
50+
id: "batch_1234",
51+
});
52+
});
53+
54+
it("ignores a key that was not scoped, instead of throwing", async () => {
55+
const service = createService();
56+
const unscopedKey = randomUUID();
57+
58+
await expect(
59+
service.saveRequest("trigger", unscopedKey, { id: "run_1234" })
60+
).resolves.toBeUndefined();
61+
await expect(service.checkRequest("trigger", unscopedKey)).resolves.toBeUndefined();
62+
});
63+
});

‎apps/webapp/test/routesBatchGetReplicaLag.guard.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ import { action as actionCreateBatch } from "~/routes/api.v3.batches";
114114
// A cuid (25 chars after the `batch_` prefix) classifies LEGACY, so the batch is owned by the legacy
115115
// (control-plane) store; the client-less reads then land on the LEGACY leg.
116116
const CUID_25 = "c".repeat(25);
117+
// Must be a v4 UUID or the route ignores the header.
118+
const REQUEST_IDEMPOTENCY_KEY = "b3f1e9c2-5d7a-4a1b-9c8e-2f6d4a7b1c30";
117119

118120
let seq = 0;
119121

@@ -339,7 +341,7 @@ describe("routes-batch-get callers under a lagging replica", () => {
339341
"content-type": "application/json",
340342
"content-length": String(Buffer.byteLength(body)),
341343
authorization: `Bearer ${seed.environment.apiKey}`,
342-
"x-trigger-request-idempotency-key": "req_idem_tb",
344+
"x-trigger-request-idempotency-key": REQUEST_IDEMPOTENCY_KEY,
343345
},
344346
body,
345347
}),

0 commit comments

Comments
 (0)