diff --git a/packages/ai-llm-proxy/src/index.ts b/packages/ai-llm-proxy/src/index.ts index 7e0e1adaa..9d845239c 100644 --- a/packages/ai-llm-proxy/src/index.ts +++ b/packages/ai-llm-proxy/src/index.ts @@ -209,7 +209,15 @@ export function isOriginAllowed(origin: string | undefined, expected: string): b return true } +/** + * Writes a JSON response, but no-ops when the client has already + * disconnected (`res.writableEnded`/`res.destroyed`) — writing to a response + * whose underlying socket is gone can emit an 'error' event on `res`, which + * would crash the process without a listener. See the `res.on('error', ...)` + * guard in `handleRequest` for the belt-and-suspenders backstop. + */ function sendJson(res: http.ServerResponse, status: number, body: unknown): void { + if (res.writableEnded || res.destroyed) return const payload = JSON.stringify(body) res.writeHead(status, { 'Content-Type': 'application/json' }) res.end(payload) @@ -219,7 +227,27 @@ function sendJson(res: http.ServerResponse, status: number, body: unknown): void // Request handler // --------------------------------------------------------------------------- -async function handleRequest(req: http.IncomingMessage, res: http.ServerResponse): Promise { +export async function handleRequest( + req: http.IncomingMessage, + res: http.ServerResponse +): Promise { + // Defensive: once the client has disconnected (see the 'close' listener + // below), writing to `res` can emit an 'error' event on the response + // stream. Without a listener, an unhandled 'error' event on a stream + // crashes the whole process, so this is a no-op safety net regardless of + // where a write happens below. + res.on('error', () => {}) + + // Tracks whether the client is still there to receive a response, and + // lets us abort the outbound LLM fetch the moment it isn't. Node emits + // 'close' on the request once the underlying connection is terminated — + // whether that's a clean end, the client's own AbortSignal.timeout firing, + // or the tab/browser dropping the connection — so this is the one signal + // we need to stop doing (and paying for) work nobody will receive the + // result of. + const clientAbortController = new AbortController() + req.on('close', () => clientAbortController.abort()) + setCorsHeaders(res) // Preflight @@ -304,11 +332,15 @@ async function handleRequest(req: http.IncomingMessage, res: http.ServerResponse let llmRes: Response try { + // Two independent reasons to give up on the outbound LLM call: it's + // taking too long (upstream timeout), or the client that asked for it + // is no longer there (clientAbortController, aborted from the 'close' + // listener above). Either one should stop the request. llmRes = await fetch(`${LLM_ENDPOINT}/chat/completions`, { method: 'POST', headers: llmHeaders, body: JSON.stringify(sanitized), - signal: AbortSignal.timeout(LLM_TIMEOUT_MS) + signal: AbortSignal.any([clientAbortController.signal, AbortSignal.timeout(LLM_TIMEOUT_MS)]) }) } catch (err) { console.error('[ai-llm-proxy] LLM request error:', err) @@ -317,6 +349,7 @@ async function handleRequest(req: http.IncomingMessage, res: http.ServerResponse } const llmBody = await llmRes.text() + if (res.writableEnded || res.destroyed) return res.writeHead(llmRes.status, { 'Content-Type': llmRes.headers.get('content-type') ?? 'application/json' }) diff --git a/packages/ai-llm-proxy/tests/unit/proxy.spec.ts b/packages/ai-llm-proxy/tests/unit/proxy.spec.ts index e393fa54f..51e1e511a 100644 --- a/packages/ai-llm-proxy/tests/unit/proxy.spec.ts +++ b/packages/ai-llm-proxy/tests/unit/proxy.spec.ts @@ -1,5 +1,6 @@ -import { describe, it, expect, beforeEach } from 'vitest' +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' import { Readable } from 'node:stream' +import { EventEmitter } from 'node:events' import type http from 'node:http' import { @@ -8,7 +9,8 @@ import { rateLimitWindows, readBody, BodyTooLargeError, - isOriginAllowed + isOriginAllowed, + handleRequest } from '../../src/index.js' // --------------------------------------------------------------------------- @@ -189,3 +191,210 @@ describe('isOriginAllowed', () => { expect(isOriginAllowed(undefined, '')).toBe(true) }) }) + +// --------------------------------------------------------------------------- +// handleRequest — client-disconnect aborts the outbound LLM fetch +// --------------------------------------------------------------------------- + +/** + * A minimal http.IncomingMessage double: an EventEmitter carrying the + * headers/method/url handleRequest reads, plus manual 'data'/'end'/'close' + * emission so tests can drive the request lifecycle explicitly (a real + * Readable's autoDestroy would fire 'close' right after 'end', which would + * defeat the point of testing 'close' as a distinct disconnect signal). + */ +function makeMockReq(overrides: Partial = {}): http.IncomingMessage { + const req = new EventEmitter() as unknown as http.IncomingMessage + Object.assign(req, { + method: 'POST', + url: '/v1/chat/completions', + headers: { authorization: 'Bearer test-token' }, + ...overrides + }) + return req +} + +/** + * A minimal http.ServerResponse double. `writableEnded`/`destroyed` mirror + * the real flags `sendJson`/`handleRequest` check before writing; `end` + * flips `writableEnded` the way a real response would. + */ +function makeMockRes(): http.ServerResponse & { writeHead: ReturnType } { + const res = new EventEmitter() as unknown as http.ServerResponse & { + writeHead: ReturnType + } + Object.assign(res, { + statusCode: 200, + headersSent: false, + writableEnded: false, + destroyed: false, + setHeader: vi.fn(), + writeHead: vi.fn(), + write: vi.fn(), + end: vi.fn(function end(this: typeof res) { + this.writableEnded = true + }) + }) + return res +} + +/** Emulates fetch's real abort semantics: rejects once the signal aborts. */ +function pendingUntilAborted(signal: AbortSignal): Promise { + return new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => { + const err = new Error('This operation was aborted') + err.name = 'AbortError' + reject(err) + }) + }) +} + +/** + * handleRequest awaits two OIDC round-trips (discovery + userinfo) before it + * ever calls `readBody`, which is what attaches the 'data'/'end' listeners + * `req.emit(...)` needs a live listener for. Polling for the listener + * (rather than guessing a fixed number of microtask ticks) keeps the test + * from being coupled to handleRequest's exact internal await count. + */ +function waitForListener(emitter: EventEmitter, event: string): Promise { + return new Promise((resolve) => { + const check = () => { + if (emitter.listenerCount(event) > 0) { + resolve() + } else { + setImmediate(check) + } + } + check() + }) +} + +describe('handleRequest — aborts the upstream LLM fetch on client disconnect', () => { + afterEach(() => { + vi.unstubAllGlobals() + rateLimitWindows.clear() + }) + + it('aborts the outbound fetch signal when the client closes the connection mid-flight', async () => { + let capturedSignal: AbortSignal | undefined + let chatCompletionsCalled = false + + vi.stubGlobal( + 'fetch', + vi.fn((url: string, init?: RequestInit) => { + if (url.includes('.well-known/openid-configuration')) { + return Promise.resolve({ + ok: true, + json: async () => ({ userinfo_endpoint: 'https://ocis.example.test/userinfo' }) + }) + } + if (url.includes('/userinfo')) { + return Promise.resolve({ ok: true, json: async () => ({ sub: 'user-close-test' }) }) + } + if (url.includes('/chat/completions')) { + capturedSignal = init?.signal as AbortSignal + chatCompletionsCalled = true + return pendingUntilAborted(capturedSignal) + } + return Promise.reject(new Error(`unexpected fetch url: ${url}`)) + }) + ) + + const req = makeMockReq() + const res = makeMockRes() + + const pending = handleRequest(req, res) + + // handleRequest awaits OIDC discovery + userinfo before it ever attaches + // the body listeners, so wait for readBody's 'data' listener before + // emitting the body. + await waitForListener(req, 'data') + req.emit('data', Buffer.from(JSON.stringify({ model: 'test-model', messages: [] }))) + req.emit('end') + + // Wait until the outbound LLM fetch has actually been issued, then + // simulate the client disconnecting while it's still in flight. + await vi.waitFor(() => { + if (!chatCompletionsCalled) throw new Error('chat/completions not called yet') + }) + req.emit('close') + + await pending + + expect(capturedSignal).toBeDefined() + expect(capturedSignal?.aborted).toBe(true) + }) + + it('does not attempt to write to the response once the client has disconnected', async () => { + let chatCompletionsCalled = false + + vi.stubGlobal( + 'fetch', + vi.fn((url: string, init?: RequestInit) => { + if (url.includes('.well-known/openid-configuration')) { + return Promise.resolve({ + ok: true, + json: async () => ({ userinfo_endpoint: 'https://ocis.example.test/userinfo' }) + }) + } + if (url.includes('/userinfo')) { + return Promise.resolve({ ok: true, json: async () => ({ sub: 'user-write-test' }) }) + } + if (url.includes('/chat/completions')) { + chatCompletionsCalled = true + return pendingUntilAborted(init?.signal as AbortSignal) + } + return Promise.reject(new Error(`unexpected fetch url: ${url}`)) + }) + ) + + const req = makeMockReq() + const res = makeMockRes() + + const pending = handleRequest(req, res) + + await waitForListener(req, 'data') + req.emit('data', Buffer.from(JSON.stringify({ model: 'test-model', messages: [] }))) + req.emit('end') + + await vi.waitFor(() => { + if (!chatCompletionsCalled) throw new Error('chat/completions not called yet') + }) + + // The client's socket is gone: mark the response as no longer writable + // (mirrors what happens to a real http.ServerResponse sharing the same + // dead connection) and signal the disconnect. + res.writableEnded = true + res.destroyed = true + req.emit('close') + + // Should resolve cleanly — no unhandled rejection/exception — and must + // not have attempted to write the 502 "Could not reach LLM endpoint" + // fallback to a response nobody can receive. + await expect(pending).resolves.toBeUndefined() + expect(res.writeHead).not.toHaveBeenCalled() + expect(res.end).not.toHaveBeenCalled() + }) + + it('attaches an error listener to the response, so a late write-after-close error cannot crash the process', async () => { + vi.stubGlobal( + 'fetch', + vi.fn((url: string) => { + // Fails fast (missing auth header) so handleRequest returns quickly + // — this test only cares about the defensive res.on('error') guard + // that's installed unconditionally at the top of handleRequest. + return Promise.reject(new Error(`unexpected fetch url: ${url}`)) + }) + ) + + const req = makeMockReq({ headers: {} }) + const res = makeMockRes() + + await handleRequest(req, res) + + // Node's EventEmitter throws synchronously when 'error' is emitted with + // no listener attached — asserting this does NOT throw proves + // handleRequest's `res.on('error', () => {})` guard is in place. + expect(() => res.emit('error', new Error('write after close'))).not.toThrow() + }) +}) diff --git a/packages/web-app-ai-data-insights-sidebar/src/composables/useLLM.ts b/packages/web-app-ai-data-insights-sidebar/src/composables/useLLM.ts index a5670c868..b6a53372e 100644 --- a/packages/web-app-ai-data-insights-sidebar/src/composables/useLLM.ts +++ b/packages/web-app-ai-data-insights-sidebar/src/composables/useLLM.ts @@ -55,7 +55,10 @@ export function useLLM(cfg: LLMConfig | null): UseLLMReturn { const r = await fetch(`${base}/chat/completions`, { method: 'POST', headers: buildHeaders(), - signal: AbortSignal.timeout(60_000), + // Generous safety-net ceiling, decoupled from the proxy's own (separately + // configurable) upstream timeout; only guards against the network or proxy + // never responding at all. + signal: AbortSignal.timeout(300_000), body: JSON.stringify({ model: cfg.model, messages, diff --git a/packages/web-app-ai-doc-summary/src/composables/useSummary.ts b/packages/web-app-ai-doc-summary/src/composables/useSummary.ts index ce7d2dce8..de35a62be 100644 --- a/packages/web-app-ai-doc-summary/src/composables/useSummary.ts +++ b/packages/web-app-ai-doc-summary/src/composables/useSummary.ts @@ -147,7 +147,10 @@ export function useSummary( const res = await fetch(`${base}/chat/completions`, { method: 'POST', headers: buildHeaders(), - signal: AbortSignal.timeout(30_000), + // Generous safety-net ceiling, decoupled from the proxy's own (separately + // configurable) upstream timeout; only guards against the network or proxy + // never responding at all. + signal: AbortSignal.timeout(300_000), body: JSON.stringify({ model: cfg.model, messages: [ diff --git a/packages/web-app-ai-folder-brief-sidebar/src/composables/useFolderBrief.ts b/packages/web-app-ai-folder-brief-sidebar/src/composables/useFolderBrief.ts index fdba22907..4298d6601 100644 --- a/packages/web-app-ai-folder-brief-sidebar/src/composables/useFolderBrief.ts +++ b/packages/web-app-ai-folder-brief-sidebar/src/composables/useFolderBrief.ts @@ -169,7 +169,10 @@ export function useFolderBrief( const res = await fetch(`${base}/chat/completions`, { method: 'POST', headers: buildHeaders(), - signal: AbortSignal.timeout(30_000), + // Generous safety-net ceiling, decoupled from the proxy's own (separately + // configurable) upstream timeout; only guards against the network or proxy + // never responding at all. + signal: AbortSignal.timeout(300_000), body: JSON.stringify({ model: cfg.model, messages: [{ role: 'user', content: prompt }], diff --git a/packages/web-app-ai-folder-readme-generator/src/composables/useLLM.ts b/packages/web-app-ai-folder-readme-generator/src/composables/useLLM.ts index b1c85219f..204abf3d0 100644 --- a/packages/web-app-ai-folder-readme-generator/src/composables/useLLM.ts +++ b/packages/web-app-ai-folder-readme-generator/src/composables/useLLM.ts @@ -70,7 +70,10 @@ export function useLLM(cfg: LLMConfig | null): UseLLMReturn { const r = await fetch(`${base}/chat/completions`, { method: 'POST', headers: buildHeaders(), - signal: AbortSignal.timeout(60_000), + // Generous safety-net ceiling, decoupled from the proxy's own (separately + // configurable) upstream timeout; only guards against the network or proxy + // never responding at all. + signal: AbortSignal.timeout(300_000), body: JSON.stringify({ model: cfg.model, messages, @@ -91,7 +94,10 @@ export function useLLM(cfg: LLMConfig | null): UseLLMReturn { const r = await fetch(`${base}/chat/completions`, { method: 'POST', headers: buildHeaders(), - signal: AbortSignal.timeout(60_000), + // Generous safety-net ceiling, decoupled from the proxy's own (separately + // configurable) upstream timeout; only guards against the network or proxy + // never responding at all. + signal: AbortSignal.timeout(300_000), body: JSON.stringify({ model: cfg.model, messages, stream: true, max_tokens: 1024 }) }) if (!r.ok) throw new Error(`LLM stream failed: ${r.status}`) diff --git a/packages/web-app-ai-image-alt-text-sidebar/src/composables/useAltText.ts b/packages/web-app-ai-image-alt-text-sidebar/src/composables/useAltText.ts index fbf554c39..ad6893f5a 100644 --- a/packages/web-app-ai-image-alt-text-sidebar/src/composables/useAltText.ts +++ b/packages/web-app-ai-image-alt-text-sidebar/src/composables/useAltText.ts @@ -193,7 +193,10 @@ export function useAltText( const res = await fetch(`${base}/chat/completions`, { method: 'POST', headers: buildHeaders(cfg.endpoint), - signal: AbortSignal.timeout(30_000), + // Generous safety-net ceiling, decoupled from the proxy's own (separately + // configurable) upstream timeout; only guards against the network or proxy + // never responding at all. + signal: AbortSignal.timeout(300_000), body: JSON.stringify({ model: cfg.model, messages: [ diff --git a/packages/web-app-ai-multi-doc-synthesizer/src/composables/useLLM.ts b/packages/web-app-ai-multi-doc-synthesizer/src/composables/useLLM.ts index b7f5992ce..ae058950c 100644 --- a/packages/web-app-ai-multi-doc-synthesizer/src/composables/useLLM.ts +++ b/packages/web-app-ai-multi-doc-synthesizer/src/composables/useLLM.ts @@ -91,7 +91,10 @@ export function useLLM(cfg: LLMConfig): UseLLMReturn { const r = await fetch(`${base}/chat/completions`, { method: 'POST', headers: buildHeaders(), - signal: AbortSignal.timeout(60_000), + // Generous safety-net ceiling, decoupled from the proxy's own (separately + // configurable) upstream timeout; only guards against the network or proxy + // never responding at all. + signal: AbortSignal.timeout(300_000), body: JSON.stringify({ model: cfg.model, messages, diff --git a/packages/web-app-ai-quick-draft-creator/src/composables/useLLM.ts b/packages/web-app-ai-quick-draft-creator/src/composables/useLLM.ts index e73fecc43..25808c0cc 100644 --- a/packages/web-app-ai-quick-draft-creator/src/composables/useLLM.ts +++ b/packages/web-app-ai-quick-draft-creator/src/composables/useLLM.ts @@ -59,17 +59,30 @@ export function useLLM(cfg: LLMConfig): UseLLMReturn { ) } - const r = await fetch(`${base}/chat/completions`, { - method: 'POST', - headers: buildHeaders(), - signal: AbortSignal.timeout(60_000), - body: JSON.stringify({ - model: cfg.model, - messages, - max_tokens: opts.maxTokens ?? 2048, - temperature: opts.temperature ?? 0.7 + let r: Response + try { + r = await fetch(`${base}/chat/completions`, { + method: 'POST', + headers: buildHeaders(), + // Generous safety-net ceiling, decoupled from the proxy's own (separately + // configurable) upstream timeout; only guards against the network or proxy + // never responding at all. + signal: AbortSignal.timeout(300_000), + body: JSON.stringify({ + model: cfg.model, + messages, + max_tokens: opts.maxTokens ?? 2048, + temperature: opts.temperature ?? 0.7 + }) }) - }) + } catch (err) { + // A timeout abort rejects the fetch itself, before there's any Response to + // check .ok on — so this has to be caught here rather than below. + if (err instanceof DOMException && err.name === 'TimeoutError') { + throw new Error($gettext('The AI service did not respond in time. Please try again later.')) + } + throw err + } if (!r.ok) { const status = r.status diff --git a/packages/web-app-ai-sensitive-data-scanner/src/composables/useLlm.ts b/packages/web-app-ai-sensitive-data-scanner/src/composables/useLlm.ts index 5e0fbd8ca..0afc60d2a 100644 --- a/packages/web-app-ai-sensitive-data-scanner/src/composables/useLlm.ts +++ b/packages/web-app-ai-sensitive-data-scanner/src/composables/useLlm.ts @@ -34,7 +34,10 @@ export function useLlm(initialConfig: LlmConfig | null) { const res = await fetch(`${base}/chat/completions`, { method: 'POST', headers: buildHeaders(), - signal: AbortSignal.timeout(30_000), + // Generous safety-net ceiling, decoupled from the proxy's own (separately + // configurable) upstream timeout; only guards against the network or proxy + // never responding at all. + signal: AbortSignal.timeout(300_000), body: JSON.stringify({ model: cfg.model, messages, diff --git a/packages/web-app-ai-sensitive-data-scanner/src/composables/useScanner.ts b/packages/web-app-ai-sensitive-data-scanner/src/composables/useScanner.ts index 414d13573..3e673436a 100644 --- a/packages/web-app-ai-sensitive-data-scanner/src/composables/useScanner.ts +++ b/packages/web-app-ai-sensitive-data-scanner/src/composables/useScanner.ts @@ -120,6 +120,13 @@ export function useScanner(llmConfig: LlmConfig | null, resources: Ref const res = await fetch(`${base}/chat/completions`, { method: 'POST', headers: buildHeaders(), - signal: AbortSignal.timeout(30_000), + // Generous safety-net ceiling, decoupled from the proxy's own (separately + // configurable) upstream timeout; only guards against the network or proxy + // never responding at all. + signal: AbortSignal.timeout(300_000), body: JSON.stringify({ model: config.model, messages: [{ role: 'user', content: prompt }],