mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-20 14:12:59 +03:00
c545855b26
Diagnosed while chasing the reused-output-index incident (see 705ac7335 / OpenClaw issue #123342): every call-log artifact showed a wire-clean response, even for requests that actually failed, because withEarlyStreamKeepalive injects its startup/keepalive/error frames directly into the outer response stream, entirely outside the request handler's own reqLogger. reqLogger.appendConvertedChunk (which populates pipeline.streamChunks.client) never sees those bytes — only what chatCore.ts's own SSE writer produced. The persisted artifact was answering "what did the handler generate," not "what did the client actually receive," which is the wrong question when diagnosing a client-visible stream defect. withEarlyStreamKeepalive wraps the handler's Promise from OUTSIDE its call tree; the reqLogger it needs to feed is created deep inside chatCore.ts, after routing/model/provider resolution, and doesn't exist yet when the keepalive frames are written. The two sides share no reference — only an identifier, if one is deliberately threaded through both. Fix: responses/route.ts now generates a correlationId before calling handleChat, passes it as handleChat's existing (already-supported, previously-unused-here) 4th positional arg — which chatCore.ts already threads into trackPendingRequest's metadata as entry.correlationId, zero changes needed there — and also into withEarlyStreamKeepalive's options. The wrapper buffers every direct- to-client write (startup frame, periodic ticks, in-band error frames) via the new earlyKeepaliveByteBuffer module, keyed by that same id. chatCore/attemptLogging.ts, which already has correlationId in scope right where it assembles the final pipeline payload before saveCallLog, takes the buffered bytes and prepends them into streamChunks.client in send order. The verbatim-forwarded real response body is deliberately NOT re-recorded here — the handler's own reqLogger already captures that; recording it twice would duplicate it in the artifact. The buffer is consumed exactly once per correlationId and swept on a 10-minute TTL so a request that never reaches the persist call (aborted, detailed logging disabled, a route that doesn't opt in) cannot leak entries forever. Scoped to /v1/responses only, where the incident actually happened. /v1/chat/completions and /v1/messages call withEarlyStreamKeepalive the same way and would need the identical two-line route change to opt in; left as a follow-up rather than bundled in sight-unseen. Test plan: - tests/unit/early-keepalive-byte-buffer.test.ts (new): record/take ordering, single-consumption, per-id isolation, empty-input no-ops, unbounded-growth cap - tests/unit/early-stream-keepalive.test.ts: two new tests — a correlationId records the startup frame and keepalive ticks but NOT the forwarded body; omitting correlationId is a true no-op - tests/unit/attempt-logging-early-keepalive-merge.test.ts (new): real temp-DB end-to-end proof against the actual persisted call-log row — early bytes prepended in send order, consumed exactly once, no-op without a correlationId, gated by detailedLoggingEnabled matching the existing streamChunks capture gate - tests/unit/chatcore-attempt-logging.test.ts (existing): unchanged, still passing — confirms the merge addition doesn't disturb existing persistence behavior - 44 passed total across the above plus earlyStreamKeepalive.test.ts, 2 pre-existing skips unrelated to this change - tsgo --noEmit: clean on all touched files
59 lines
2.1 KiB
TypeScript
59 lines
2.1 KiB
TypeScript
/**
|
|
* @file earlyKeepaliveByteBuffer.ts
|
|
* @description Bridges bytes withEarlyStreamKeepalive writes directly to the
|
|
* client (outside the request handler's own reqLogger) back into that same
|
|
* request's persisted call-log artifact.
|
|
*
|
|
* withEarlyStreamKeepalive wraps a route's handler Promise from OUTSIDE the
|
|
* handler's own call tree — it has no reference to the reqLogger the handler
|
|
* creates deep inside chatCore.ts, and by the time that reqLogger exists the
|
|
* keepalive/startup frames may already be written. A shared correlationId
|
|
* (threaded by the route as handleChat's 4th positional arg, and separately
|
|
* into withEarlyStreamKeepalive's options) is the only thing both sides
|
|
* share, so recordEarlyKeepaliveBytes/takeEarlyKeepaliveBytes key on that
|
|
* instead of trying to pass a live object reference across the boundary.
|
|
*
|
|
* Entries are consumed once (chatCore/attemptLogging.ts calls
|
|
* takeEarlyKeepaliveBytes exactly when it assembles the final call-log
|
|
* payload) and swept on a TTL so a request that never reaches that point
|
|
* (aborted, detailed logging disabled, a route that never wires this up)
|
|
* cannot leak buffered bytes forever.
|
|
*/
|
|
|
|
const MAX_ITEMS_PER_CORRELATION = 200;
|
|
const ENTRY_TTL_MS = 10 * 60 * 1000;
|
|
|
|
type BufferEntry = { chunks: string[]; createdAt: number };
|
|
|
|
const buffers = new Map<string, BufferEntry>();
|
|
|
|
function sweepExpired(): void {
|
|
const cutoff = Date.now() - ENTRY_TTL_MS;
|
|
for (const [correlationId, entry] of buffers) {
|
|
if (entry.createdAt < cutoff) {
|
|
buffers.delete(correlationId);
|
|
}
|
|
}
|
|
}
|
|
|
|
export function recordEarlyKeepaliveBytes(correlationId: string, chunk: string): void {
|
|
if (!correlationId || !chunk) return;
|
|
sweepExpired();
|
|
let entry = buffers.get(correlationId);
|
|
if (!entry) {
|
|
entry = { chunks: [], createdAt: Date.now() };
|
|
buffers.set(correlationId, entry);
|
|
}
|
|
if (entry.chunks.length < MAX_ITEMS_PER_CORRELATION) {
|
|
entry.chunks.push(chunk);
|
|
}
|
|
}
|
|
|
|
export function takeEarlyKeepaliveBytes(correlationId: string): string[] {
|
|
sweepExpired();
|
|
const entry = buffers.get(correlationId);
|
|
if (!entry) return [];
|
|
buffers.delete(correlationId);
|
|
return entry.chunks;
|
|
}
|