fix(backend): generation-aware SSE event ids — stale cursors can no longer eat events
Audit finding 3.1 (high, 3/3 reviewer consensus). Event ids restart at 1 when the backend restarts; a browser auto-reconnect carrying the old numeric Last-Event-ID was honored whenever the new process had already emitted that many events, silently suppressing fresh events (same ids, different content). The previous guard only caught cursor > lastId. Wire ids are now "<generation>:<seq>" (generation = per-hub instance token; seq = the existing per-session monotonic counter). The hub parses raw header/query candidates itself: other-generation and legacy bare- number cursors are stale → replay from the beginning; same-generation cursors keep the newest-valid-wins behavior. EventSource treats ids as opaque, so no frontend change. Finding 3.2 (eviction) resolved by NOT evicting: close keeps the seq counter on purpose (sessions reopen; monotonicity is what makes old cursors detectable) — documented at the call site; buffers are emptied by clear() and ring-bounded at 200. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -16,16 +16,6 @@ const RESUME_FAILURE_MESSAGE =
|
||||
const DWH_UNREACHABLE_MESSAGE =
|
||||
"Cannot start a session: the data warehouse is unreachable. Check the VPN connection and try again.";
|
||||
|
||||
function eventCursor(...values: unknown[]): number {
|
||||
let cursor = 0;
|
||||
for (const value of values.flatMap((item) => Array.isArray(item) ? item : [item])) {
|
||||
if (typeof value !== "string" || !/^\d+$/.test(value)) continue;
|
||||
const parsed = Number(value);
|
||||
if (Number.isSafeInteger(parsed)) cursor = Math.max(cursor, parsed);
|
||||
}
|
||||
return cursor;
|
||||
}
|
||||
|
||||
export function sessionRoutes(
|
||||
app: FastifyInstance,
|
||||
d: {
|
||||
@@ -370,6 +360,9 @@ export function sessionRoutes(
|
||||
} catch {
|
||||
return storageFailure(reply);
|
||||
} finally {
|
||||
// clear, NOT forget: a closed session can be reopened, and the per-session seq
|
||||
// monotonicity is what keeps a browser's old cursor detectable. The buffer is
|
||||
// emptied here; only delete discards the id counter.
|
||||
d.hub.clear(id);
|
||||
}
|
||||
return { closed: true };
|
||||
@@ -383,10 +376,6 @@ export function sessionRoutes(
|
||||
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
|
||||
} catch { return storageFailure(reply); }
|
||||
const rt = d.mgr.get(id);
|
||||
const afterId = eventCursor(
|
||||
req.headers["last-event-id"],
|
||||
(req.query as { lastEventId?: unknown }).lastEventId,
|
||||
);
|
||||
// Add CORS headers manually: reply.raw.writeHead bypasses Fastify's onSend hooks
|
||||
// (where @fastify/cors injects headers), so we must set them explicitly here.
|
||||
const origin = (req.headers.origin as string | undefined) ?? "*";
|
||||
@@ -401,10 +390,12 @@ export function sessionRoutes(
|
||||
// Send the handshake immediately. Without this, Node waits for the first event body and
|
||||
// proxies/clients cannot establish an idle SSE subscription or inspect its headers.
|
||||
reply.raw.flushHeaders();
|
||||
const send = (event: string, data: object, eventId: number) =>
|
||||
const send = (event: string, data: object, eventId: string) =>
|
||||
reply.raw.write(`id: ${eventId}\nevent: ${event}\ndata: ${JSON.stringify(data)}\n\n`);
|
||||
const off = d.hub.subscribe(id, send, {
|
||||
afterId,
|
||||
// Raw cursor candidates: the hub parses "<generation>:<seq>" and treats any
|
||||
// other-generation (or legacy numeric) cursor as stale → replay from the start.
|
||||
after: [req.headers["last-event-id"], (req.query as { lastEventId?: unknown }).lastEventId],
|
||||
pending: rt?.bridge.pendingWidget() ?? null,
|
||||
// clear()/forget() end every old transport so native EventSource reconnects with its
|
||||
// Last-Event-ID instead of remaining attached to a subscriber callback that no longer exists.
|
||||
|
||||
+42
-18
@@ -1,4 +1,4 @@
|
||||
type Send = (event: string, data: object, id: number) => void;
|
||||
type Send = (event: string, data: object, id: string) => void;
|
||||
|
||||
interface Subscriber {
|
||||
send: Send;
|
||||
@@ -9,8 +9,13 @@ interface Subscriber {
|
||||
/**
|
||||
* Per-session ring buffer of recent events.
|
||||
*
|
||||
* Event ids are transport identity: they are monotonically increasing for the lifetime of a
|
||||
* session id, including across a cold Resume that clears the old buffer and subscribers.
|
||||
* Event ids are transport identity: `<generation>:<seq>`, where seq is monotonically
|
||||
* increasing for the lifetime of a session id in THIS process (including across a cold
|
||||
* Resume that clears the old buffer and subscribers), and generation identifies the hub
|
||||
* instance. After a backend restart seqs start over from 1: without the generation part,
|
||||
* a browser auto-reconnect carrying an old numeric cursor would silently suppress the new
|
||||
* process's first events (same ids, different content). A cursor whose generation does not
|
||||
* match this instance is stale by definition and replays from the beginning.
|
||||
*/
|
||||
const BUFFER_LIMIT = 200;
|
||||
|
||||
@@ -21,7 +26,8 @@ interface BufferedEvent {
|
||||
}
|
||||
|
||||
interface SubscribeOptions {
|
||||
afterId?: number;
|
||||
/** Raw cursor candidates (Last-Event-ID header, query param) — parsed by the hub. */
|
||||
after?: unknown[];
|
||||
pending?: object | null;
|
||||
close?: () => void;
|
||||
}
|
||||
@@ -42,6 +48,32 @@ export class SseHub {
|
||||
private subs = new Map<string, Set<Subscriber>>();
|
||||
private buffers = new Map<string, BufferedEvent[]>();
|
||||
private lastIds = new Map<string, number>();
|
||||
private readonly generation =
|
||||
`${Date.now().toString(36)}${Math.random().toString(36).slice(2, 6)}`;
|
||||
|
||||
private wireId(seq: number): string {
|
||||
return `${this.generation}:${seq}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the raw cursor candidates (header + query) to a seq in THIS generation,
|
||||
* taking the newest valid one. A cursor from another generation (or the legacy
|
||||
* bare-number format) is stale → 0, i.e. replay from the beginning.
|
||||
*/
|
||||
private parseAfter(sessionId: string, candidates: unknown[] = []): number {
|
||||
let after = 0;
|
||||
for (const value of candidates.flatMap((item) => (Array.isArray(item) ? item : [item]))) {
|
||||
if (typeof value !== "string") continue;
|
||||
const m = /^([^:]+):(\d+)$/.exec(value);
|
||||
if (!m || m[1] !== this.generation) continue;
|
||||
const seq = Number(m[2]);
|
||||
// Same generation, but a seq this session never produced cannot identify an event.
|
||||
if (Number.isSafeInteger(seq) && seq <= (this.lastIds.get(sessionId) ?? 0)) {
|
||||
after = Math.max(after, seq);
|
||||
}
|
||||
}
|
||||
return after;
|
||||
}
|
||||
|
||||
subscribe(
|
||||
sessionId: string,
|
||||
@@ -52,18 +84,10 @@ export class SseHub {
|
||||
const subscriber = { send, close: options.close ?? (() => undefined), closed: false };
|
||||
this.subs.get(sessionId)!.add(subscriber);
|
||||
|
||||
const requestedAfterId = Number.isSafeInteger(options.afterId) && (options.afterId ?? 0) >= 0
|
||||
? options.afterId ?? 0
|
||||
: 0;
|
||||
// Native EventSource persists Last-Event-ID across a backend process restart. A cursor newer
|
||||
// than anything this hub generation has produced cannot identify an event in this process,
|
||||
// so replay the fresh generation from its beginning instead of suppressing every low id.
|
||||
const afterId = requestedAfterId > (this.lastIds.get(sessionId) ?? 0)
|
||||
? 0
|
||||
: requestedAfterId;
|
||||
const afterId = this.parseAfter(sessionId, options.after);
|
||||
const buf = this.buffers.get(sessionId) ?? [];
|
||||
for (const item of buf) {
|
||||
if (item.id > afterId) send(item.event, item.data, item.id);
|
||||
if (item.id > afterId) send(item.event, item.data, this.wireId(item.id));
|
||||
}
|
||||
|
||||
// A pending gate normally already exists in the buffer. If ring-buffer eviction removed it,
|
||||
@@ -74,18 +98,18 @@ export class SseHub {
|
||||
if (pendingId !== null && !pendingBuffered) {
|
||||
const data = { type: "ui_request", ui_request: options.pending! };
|
||||
const item = this.buffer(sessionId, "ui_request", data);
|
||||
send(item.event, item.data, item.id);
|
||||
send(item.event, item.data, this.wireId(item.id));
|
||||
}
|
||||
|
||||
return () => this.subs.get(sessionId)?.delete(subscriber);
|
||||
}
|
||||
|
||||
publish(sessionId: string, event: string, data: object): number {
|
||||
publish(sessionId: string, event: string, data: object): string {
|
||||
const item = this.buffer(sessionId, event, data);
|
||||
for (const subscriber of this.subs.get(sessionId) ?? []) {
|
||||
subscriber.send(event, data, item.id);
|
||||
subscriber.send(event, data, this.wireId(item.id));
|
||||
}
|
||||
return item.id;
|
||||
return this.wireId(item.id);
|
||||
}
|
||||
|
||||
private buffer(sessionId: string, event: string, data: object): BufferedEvent {
|
||||
|
||||
Reference in New Issue
Block a user