Skip to content

Commit 9e9bae0

Browse files
d-csTrigger.dev RepoOps
authored andcommitted
fix(webapp): retry S2 stream creation conflicts
Fix intermittent realtime read and subscription errors when concurrent readers or writers first create a stream. Retry stream-creation conflicts within bounded limits while preserving the requested position. Mono-RevId: 066b14085d2ca9eac94b01577010f7a9a433879c
1 parent f4dc2b9 commit 9e9bae0

4 files changed

Lines changed: 187 additions & 25 deletions

File tree

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+
Fix intermittent read and subscription errors when multiple clients first open the same realtime stream.

‎apps/webapp/app/services/realtime/s2realtimeStreams.server.ts‎

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { LogLevel } from "@trigger.dev/core/logger";
55
import { Logger } from "@trigger.dev/core/logger";
66
import { headerValue } from "@trigger.dev/core/v3";
77
import { randomUUID } from "node:crypto";
8+
import { setTimeout as sleep } from "node:timers/promises";
89
import { ServiceValidationError } from "~/v3/services/common.server";
910

1011
// S2's per-record metered-size limit. Verified empirically against
@@ -332,7 +333,7 @@ export class S2RealtimeStreams implements StreamResponder, StreamIngestor {
332333

333334
/**
334335
* Non-blocking read of the records after `afterSeqNum`. Retries transient
335-
* failures (network errors, 5xx, and reads S2 ends with an `error` event)
336+
* failures (network errors, creation conflicts, 5xx, and reads S2 ends with an `error` event)
336337
* because the `.wait()` race check depends on this read to find a record
337338
* sent before the waitpoint registered.
338339
*/
@@ -345,7 +346,8 @@ export class S2RealtimeStreams implements StreamResponder, StreamIngestor {
345346
return await this.#readRecordsOnce(s2Stream, afterSeqNum);
346347
} catch (error) {
347348
const status = (error as { status?: number }).status;
348-
const isClientError = status !== undefined && status >= 400 && status < 500;
349+
const isClientError =
350+
status !== undefined && status >= 400 && status < 500 && status !== 409;
349351
if (isClientError || attempt + 1 >= maxAttempts) {
350352
this.logger.error("S2 readRecords failed", {
351353
stream: s2Stream,
@@ -794,31 +796,44 @@ export class S2RealtimeStreams implements StreamResponder, StreamIngestor {
794796
if (opts.clamp != null) qs.set("clamp", String(opts.clamp));
795797
if (opts.wait != null) qs.set("wait", String(opts.wait));
796798

797-
const res = await fetch(`${this.baseUrl}/streams/${encodeURIComponent(stream)}/records?${qs}`, {
798-
method: "GET",
799-
headers: {
800-
Authorization: `Bearer ${this.token}`,
801-
Accept: "text/event-stream",
802-
"S2-Format": "raw",
803-
"S2-Basin": this.basin,
804-
},
805-
signal: opts.signal,
806-
});
799+
// Concurrent first readers/writers can race S2's automatic stream creation.
800+
// Retry only the conflict, before exposing any SSE bytes to the caller.
801+
const backoffsMs = [100, 200, 400];
802+
for (let attempt = 0; ; attempt++) {
803+
const res = await fetch(
804+
`${this.baseUrl}/streams/${encodeURIComponent(stream)}/records?${qs}`,
805+
{
806+
method: "GET",
807+
headers: {
808+
Authorization: `Bearer ${this.token}`,
809+
Accept: "text/event-stream",
810+
"S2-Format": "raw",
811+
"S2-Basin": this.basin,
812+
},
813+
signal: opts.signal,
814+
}
815+
);
807816

808-
if (!res.ok) {
809-
await cancelResponseBody(res);
810-
throw new Error(`S2 stream failed: ${res.status} ${res.statusText}`);
811-
}
817+
if (!res.ok) {
818+
await cancelResponseBody(res);
819+
const delay = backoffsMs[attempt];
820+
if (res.status === 409 && delay !== undefined) {
821+
await sleep(delay, undefined, { signal: opts.signal });
822+
continue;
823+
}
824+
throw new Error(`S2 stream failed: ${res.status} ${res.statusText}`);
825+
}
812826

813-
const headers = new Headers(res.headers);
814-
headers.set("X-Stream-Version", "v2");
815-
headers.set("Access-Control-Expose-Headers", "*");
827+
const headers = new Headers(res.headers);
828+
headers.set("X-Stream-Version", "v2");
829+
headers.set("Access-Control-Expose-Headers", "*");
816830

817-
return new Response(res.body, {
818-
headers,
819-
status: res.status,
820-
statusText: res.statusText,
821-
});
831+
return new Response(res.body, {
832+
headers,
833+
status: res.status,
834+
statusText: res.statusText,
835+
});
836+
}
822837
}
823838

824839
private parseLastEventId(lastEventId?: string): number | undefined {

‎apps/webapp/test/helpers/s2FaultProxy.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,13 @@ const HOP_BY_HOP = new Set([
1212
"content-encoding",
1313
]);
1414

15-
type Fault = { match: string; afterBatches: number; message: string; remaining: number };
15+
type Fault = {
16+
match: string;
17+
afterBatches: number;
18+
message: string;
19+
remaining: number;
20+
status?: number;
21+
};
1622

1723
/**
1824
* An HTTP proxy that sits between the webapp and s2-lite and can make one SSE
@@ -70,6 +76,11 @@ export class S2FaultProxy {
7076
await new Promise<void>((resolve) => this.server!.close(() => resolve()));
7177
}
7278

79+
/** Reject before SSE headers, as when concurrent first reads race to create a stream. */
80+
rejectNextRead(match: string, status: number, times = 1) {
81+
this.faults.push({ match, status, remaining: times, afterBatches: 0, message: "" });
82+
}
83+
7384
private takeFault(req: IncomingMessage): Fault | undefined {
7485
const accept = String(req.headers.accept ?? "");
7586
if (req.method !== "GET" || !accept.includes("text/event-stream")) return undefined;
@@ -100,6 +111,10 @@ export class S2FaultProxy {
100111
res.on("close", () => abort.abort());
101112

102113
const fault = this.takeFault(req);
114+
if (fault?.status) {
115+
res.writeHead(fault.status).end();
116+
return;
117+
}
103118
const upstream = await fetch(`${target}${req.url}`, {
104119
method: req.method,
105120
headers,
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
import { randomUUID } from "node:crypto";
2+
import { setTimeout } from "node:timers/promises";
3+
import { afterAll, beforeAll, describe, expect, it } from "vitest";
4+
import { createS2Container, type StartedS2Container } from "@internal/testcontainers/webapp";
5+
import { S2RealtimeStreams } from "~/services/realtime/s2realtimeStreams.server";
6+
import { S2FaultProxy } from "./helpers/s2FaultProxy";
7+
8+
describe("S2 subscription handshake", () => {
9+
let s2: StartedS2Container;
10+
let streams: S2RealtimeStreams;
11+
const proxy = new S2FaultProxy();
12+
13+
beforeAll(async () => {
14+
s2 = await createS2Container();
15+
const endpoint = await proxy.start();
16+
proxy.setTarget(s2.endpoint);
17+
streams = new S2RealtimeStreams({
18+
endpoint,
19+
basin: s2.basin,
20+
accessToken: "unused",
21+
s2WaitSeconds: 0,
22+
});
23+
}, 60_000);
24+
25+
afterAll(async () => {
26+
await proxy.stop();
27+
await s2?.container.stop();
28+
});
29+
30+
function subscribe(session: string, signal = AbortSignal.timeout(5_000), lastEventId?: string) {
31+
return streams.streamResponseFromSessionStream(
32+
new Request("http://localhost/session"),
33+
session,
34+
"out",
35+
signal,
36+
{ lastEventId }
37+
);
38+
}
39+
40+
it("retries creation conflicts without losing the requested resume position", async () => {
41+
const session = randomUUID();
42+
await streams.appendPartToSessionStream("first", "p0", session, "out");
43+
await streams.appendPartToSessionStream("second", "p1", session, "out");
44+
proxy.rejectNextRead(session, 409, 2);
45+
46+
const response = await subscribe(session, undefined, "0");
47+
const body = await response.text();
48+
expect(response.status).toBe(200);
49+
expect(response.headers.get("X-Stream-Version")).toBe("v2");
50+
expect(body).toContain("second");
51+
expect(body).not.toContain("first");
52+
expect(proxy.injected.filter((path) => path.includes(session))).toHaveLength(2);
53+
});
54+
55+
it("fails after four attempts if the conflict persists", async () => {
56+
const session = randomUUID();
57+
proxy.rejectNextRead(session, 409, 10);
58+
await expect(subscribe(session)).rejects.toThrow("S2 stream failed: 409");
59+
expect(proxy.injected.filter((path) => path.includes(session))).toHaveLength(4);
60+
});
61+
62+
it("retries non-blocking read conflicts without losing the requested resume position", async () => {
63+
const session = randomUUID();
64+
await streams.appendPartToSessionStream("first", "p0", session, "out");
65+
await streams.appendPartToSessionStream("second", "p1", session, "out");
66+
proxy.rejectNextRead(session, 409, 2);
67+
68+
const records = await streams.readSessionStreamRecords(session, "out", 0);
69+
70+
expect(records.map(({ data, id, seqNum }) => ({ data, id, seqNum }))).toEqual([
71+
{ data: "second", id: "p1", seqNum: 1 },
72+
]);
73+
expect(proxy.injected.filter((path) => path.includes(session))).toHaveLength(2);
74+
});
75+
76+
it.each([
77+
{ status: 409, attempts: 3 },
78+
{ status: 403, attempts: 1 },
79+
])(
80+
"bounds non-blocking reads to $attempts attempts for HTTP $status",
81+
async ({ status, attempts }) => {
82+
const run = randomUUID();
83+
proxy.rejectNextRead(run, status, 10);
84+
85+
await expect(streams.readRecords(run, "input")).rejects.toThrow(
86+
`S2 readRecords failed: ${status}`
87+
);
88+
expect(proxy.injected.filter((path) => path.includes(run))).toHaveLength(attempts);
89+
}
90+
);
91+
92+
it("opens concurrent subscriptions to previously unwritten streams", async () => {
93+
for (let round = 0; round < 8; round++) {
94+
const session = randomUUID();
95+
const statuses = await Promise.all(
96+
Array.from({ length: 8 }, async () => {
97+
const response = await subscribe(session);
98+
await response.text();
99+
return response.status;
100+
})
101+
);
102+
expect(statuses).toEqual(Array(8).fill(200));
103+
}
104+
});
105+
106+
it("does not retry other HTTP errors", async () => {
107+
const session = randomUUID();
108+
proxy.rejectNextRead(session, 403, 10);
109+
await expect(subscribe(session)).rejects.toThrow("S2 stream failed: 403");
110+
expect(proxy.injected.filter((path) => path.includes(session))).toHaveLength(1);
111+
});
112+
113+
it("stops retrying when the subscriber disconnects during backoff", async () => {
114+
const session = randomUUID();
115+
const abort = new AbortController();
116+
proxy.rejectNextRead(session, 409, 10);
117+
const result = subscribe(session, abort.signal).catch((error: unknown) => error);
118+
await expect.poll(() => proxy.injected.some((path) => path.includes(session))).toBe(true);
119+
abort.abort();
120+
expect(await result).toMatchObject({ name: "AbortError" });
121+
const attempts = proxy.injected.filter((path) => path.includes(session)).length;
122+
// Prove no new request is sent after cancellation, across the first retry delay.
123+
await setTimeout(150);
124+
expect(proxy.injected.filter((path) => path.includes(session))).toHaveLength(attempts);
125+
});
126+
});

0 commit comments

Comments
 (0)