Skip to content

Commit 89a5f66

Browse files
committed
fix(chat): preserve recovered append sequence
1 parent 8aa8866 commit 89a5f66

5 files changed

Lines changed: 132 additions & 15 deletions

File tree

apps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -181,21 +181,24 @@ const { action, loader } = createActionApiRoute(
181181
error: recoveryError,
182182
});
183183
} else if (recoveredSeq !== undefined) {
184-
const recovered = await commitSessionStreamPart(
184+
await commitSessionStreamPart(
185185
authentication.environment.id,
186186
addressingKey,
187187
params.io,
188188
partId,
189189
pendingClaim.claimValue,
190190
recoveredSeq
191191
);
192-
if (recovered) {
193-
appendSeq = recoveredSeq;
194-
claim = { status: "committed", seq: recoveredSeq };
195-
}
192+
// The S2 record is durable even if Redis expired or evicted the
193+
// pending claim before the best-effort sequence write-back.
194+
appendSeq = recoveredSeq;
195+
claim = { status: "committed", seq: recoveredSeq };
196196
}
197197

198-
if (claim.status === "pending") {
198+
// `.out` data may have been trimmed after it was accepted, so a
199+
// successful search with no record preserves the prior no-op response.
200+
// A failed search proves nothing and remains retryable for both channels.
201+
if (claim.status === "pending" && (params.io === "in" || recoveryError)) {
199202
return json(
200203
{ ok: false, error: "This append is still in progress, please retry." },
201204
{ status: 409, headers: { "Retry-After": "1" } }

apps/webapp/app/services/sessionStreamWaitpointCache.server.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,14 @@ const COMMIT_APPEND_DEDUPE_SCRIPT = `
163163
if current == ARGV[2] then
164164
return 1
165165
end
166-
if current ~= ARGV[1] then
166+
if current and current ~= ARGV[1] then
167167
return 0
168168
end
169+
local restoredMissingClaim = not current
169170
redis.call("SET", KEYS[1], ARGV[2], "EX", ARGV[3])
171+
if restoredMissingClaim then
172+
return 2
173+
end
170174
return 1
171175
`;
172176

@@ -276,7 +280,7 @@ function parsePendingClaimedAt(value: string): number | undefined {
276280
return Number.isSafeInteger(claimedAt) && claimedAt >= 0 ? claimedAt : undefined;
277281
}
278282

279-
/** Replace the exact pending claim with the committed S2 sequence. */
283+
/** Persist a committed S2 sequence if the expected claim still owns the key or the key vanished. */
280284
export async function commitSessionStreamPart(
281285
environmentId: string,
282286
addressingKey: string,
@@ -296,7 +300,7 @@ export async function commitSessionStreamPart(
296300
`${APPEND_DEDUPE_SEQUENCE_PREFIX}${seq}`,
297301
String(APPEND_DEDUPE_TTL_SECONDS)
298302
);
299-
return result === 1;
303+
return result === 1 || result === 2;
300304
} catch (error) {
301305
logger.error("Failed to commit session stream append part", {
302306
environmentId,

apps/webapp/app/v3/webhookEngine.server.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,17 +234,17 @@ function createWebhookEngine() {
234234
pendingClaim.claimedAt
235235
);
236236
if (recoveredSeq !== undefined) {
237-
const recovered = await commitSessionStreamPart(
237+
await commitSessionStreamPart(
238238
environment.id,
239239
addressingKey,
240240
"in",
241241
deliveryId,
242242
pendingClaim.claimValue,
243243
recoveredSeq
244244
);
245-
if (recovered) {
246-
claim = { status: "committed", seq: recoveredSeq };
247-
}
245+
// The S2 record is durable even if Redis expired or evicted the
246+
// pending claim before the best-effort sequence write-back.
247+
claim = { status: "committed", seq: recoveredSeq };
248248
}
249249

250250
if (claim.status === "pending") {

apps/webapp/test/helpers/sessionStream.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,9 @@ export async function appendInput(opts: {
122122
body: string;
123123
partId?: string;
124124
origin?: string;
125+
io?: "out" | "in";
125126
}): Promise<{ status: number; acao: string | null; json: unknown }> {
126-
const url = `${sessionChannelUrl(opts.baseUrl, opts.addressingKey, "in")}/append`;
127+
const url = `${sessionChannelUrl(opts.baseUrl, opts.addressingKey, opts.io ?? "in")}/append`;
127128
const res = await fetch(url, {
128129
method: "POST",
129130
headers: {

apps/webapp/test/session-stream.e2e.test.ts

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ async function setupSession() {
8282
return {
8383
addressingKey,
8484
environmentId: environment.id,
85+
apiKey,
8586
token,
8687
producer,
8788
inProducer,
@@ -478,7 +479,115 @@ describe("session stream e2e", () => {
478479
}
479480
});
480481

481-
it("E16 subscribe with an invalid token is rejected", async () => {
482+
it("E16 in/append recovery survives an expired Redis claim", async () => {
483+
const { addressingKey, environmentId, token, inProducer, baseUrl } = await setupSession();
484+
const redis = new Redis({ ...server.redis, keyPrefix: "tr:" });
485+
const monitorSource = new Redis(server.redis);
486+
let monitor: Redis | undefined;
487+
488+
try {
489+
const payload = JSON.stringify({ kind: "message", text: "expired during recovery" });
490+
const partId = `expired-${randomBytes(6).toString("hex")}`;
491+
const claimKey = `ssa:${encodeURIComponent(environmentId)}:${encodeURIComponent(
492+
addressingKey
493+
)}:in:${encodeURIComponent(partId)}`;
494+
const waitpointKey = `ssw:${environmentId}:${addressingKey}:in`;
495+
496+
await redis.set(claimKey, `pending:${Date.now()}:crashed-owner`, "EX", 5 * 60);
497+
await redis.sadd(waitpointKey, "waitpoint_expired_claim_test");
498+
const originalSeq = await inProducer.appendData(payload, partId);
499+
500+
monitor = await monitorSource.monitor();
501+
const claimRead = new Promise<void>((resolve, reject) => {
502+
const timeout = setTimeout(
503+
() => reject(new Error("Timed out waiting for claim read")),
504+
5_000
505+
);
506+
const onMonitor = (_time: string, args: string[]) => {
507+
if (args[0]?.toLowerCase() !== "get" || args[1] !== `tr:${claimKey}`) return;
508+
monitor!.off("monitor", onMonitor);
509+
void redis.del(claimKey).then(
510+
() => {
511+
clearTimeout(timeout);
512+
resolve();
513+
},
514+
(error) => {
515+
clearTimeout(timeout);
516+
reject(error);
517+
}
518+
);
519+
};
520+
monitor.on("monitor", onMonitor);
521+
});
522+
523+
const retryPromise = appendInput({ baseUrl, addressingKey, token, partId, body: payload });
524+
await claimRead;
525+
const retry = await retryPromise;
526+
527+
expect(retry.status).toBe(200);
528+
expect(retry.json).toEqual({ ok: true, seq: originalSeq });
529+
expect(await redis.get(claimKey)).toBe(`seq:${originalSeq}`);
530+
expect(await redis.exists(waitpointKey)).toBe(0);
531+
532+
const committedRetry = await appendInput({
533+
baseUrl,
534+
addressingKey,
535+
token,
536+
partId,
537+
body: payload,
538+
});
539+
expect(committedRetry.status).toBe(200);
540+
expect(committedRetry.json).toEqual({ ok: true, seq: originalSeq });
541+
542+
const { parts } = await collectSessionOut({
543+
baseUrl,
544+
addressingKey,
545+
token,
546+
io: "in",
547+
timeoutInSeconds: 1,
548+
maxMs: 5_000,
549+
});
550+
expect(parts.filter((part) => part.chunk != null)).toHaveLength(1);
551+
} finally {
552+
monitor?.disconnect();
553+
await monitorSource.quit();
554+
await redis.quit();
555+
}
556+
});
557+
558+
it("E17 out/append keeps a missing recovered record as a successful no-op", async () => {
559+
const { addressingKey, environmentId, apiKey, baseUrl } = await setupSession();
560+
const redis = new Redis({ ...server.redis, keyPrefix: "tr:" });
561+
562+
try {
563+
const payload = JSON.stringify({ kind: "message", text: "already trimmed" });
564+
const partId = `trimmed-${randomBytes(6).toString("hex")}`;
565+
const claimKey = `ssa:${encodeURIComponent(environmentId)}:${encodeURIComponent(
566+
addressingKey
567+
)}:out:${encodeURIComponent(partId)}`;
568+
const waitpointKey = `ssw:${environmentId}:${addressingKey}:out`;
569+
570+
await redis.set(claimKey, `pending:${Date.now()}:crashed-owner`, "EX", 5 * 60);
571+
await redis.sadd(waitpointKey, "waitpoint_trimmed_claim_test");
572+
573+
const retry = await appendInput({
574+
baseUrl,
575+
addressingKey,
576+
token: apiKey,
577+
partId,
578+
body: payload,
579+
io: "out",
580+
});
581+
582+
expect(retry.status).toBe(200);
583+
expect(retry.json).toEqual({ ok: true });
584+
expect(await redis.exists(waitpointKey)).toBe(0);
585+
} finally {
586+
await redis.quit();
587+
}
588+
});
589+
590+
it("E18 subscribe with an invalid token is rejected", async () => {
482591
const { addressingKey, baseUrl } = await setupSession();
483592

484593
const { status } = await openChannelRaw({

0 commit comments

Comments
 (0)