refactor(ai): address PR #176 review — finite-timeout wording, env doc, tests, permanent provider-http module
- Wording: every comment now says the stream timeouts are RAISED to a
generous-but-finite ~15-min silence timeout, not "disabled (0)" (the stale
comments contradicted the code, which uses AI_STREAM_TIMEOUT_MS, default
900000ms).
- Architecture (the load-bearing-temporary trap): the streaming fetch reached
the chat provider only by riding the "temporary DIAGNOSTIC" telemetry, so
deleting the telemetry by its own label would silently revert the timeout fix.
Legitimize it: rename ai-http-diagnostics.ts -> ai-provider-http.ts,
createDiagnosticFetch -> createInstrumentedFetch, field aiDiagnosticFetch ->
aiProviderFetch, drop the "temporary" labels, and document the chat transport
(streaming fetch + instrumentation) as one intentional construct.
- Docs: AI_STREAM_TIMEOUT_MS added to .env.example next to AI_EMBEDDING_TIMEOUT_MS.
- Tests:
- ai-provider-http.spec: createInstrumentedFetch delegates to the injected
baseFetch with the same input/init, returns the Response untouched, rethrows
the error, and defaults to global fetch — covering the baseFetch seam.
- ai-streaming-fetch.spec: the delayed-server test is now LOAD-BEARING — with
AI_STREAM_TIMEOUT_MS set below the 1.5s server delay the call actually rejects
(a lost dispatcher -> global 300s default would NOT), proving the configured
dispatcher is wired; plus the default-timeout happy path.
server tsc clean; ai-streaming-fetch / ai-provider-http / ai.service / mcp-servers
/ ai-error specs green (41).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -14,8 +14,7 @@ import { AiNotConfiguredException } from './ai-not-configured.exception';
|
||||
import { AiEmbeddingNotConfiguredException } from './ai-embedding-not-configured.exception';
|
||||
import { AiSttNotConfiguredException } from './ai-stt-not-configured.exception';
|
||||
import { describeProviderError } from './ai-error.util';
|
||||
// DIAGNOSTIC (provider ECONNRESET investigation) — temporary.
|
||||
import { createDiagnosticFetch } from './ai-http-diagnostics';
|
||||
import { createInstrumentedFetch } from './ai-provider-http';
|
||||
import { createStreamingFetch } from './ai-streaming-fetch';
|
||||
import { AiProviderCredentialsRepo } from '@docmost/db/repos/ai-chat/ai-provider-credentials.repo';
|
||||
import { SecretBoxService } from '../crypto/secret-box';
|
||||
@@ -46,12 +45,12 @@ export interface ChatModelOverride {
|
||||
export class AiService {
|
||||
private readonly logger = new Logger(AiService.name);
|
||||
|
||||
// Provider HTTP fetch for the chat path: a streaming fetch that DISABLES
|
||||
// undici's 300s headers/body timeouts (#175 — long agent turns were severed
|
||||
// mid-stream), wrapped with passive ECONNRESET-investigation telemetry so the
|
||||
// logs observe the exact transport the turn uses. Held for the service
|
||||
// lifetime to reuse the streaming dispatcher's connection pool.
|
||||
private readonly aiDiagnosticFetch = createDiagnosticFetch(
|
||||
// Provider HTTP fetch for the chat path: the streaming fetch — which RAISES
|
||||
// undici's 300s headers/body timeouts to a generous-but-finite silence timeout
|
||||
// so a long agent turn is not severed mid-stream (#175) — wrapped with the
|
||||
// provider-HTTP instrumentation so the logs observe that exact transport. Held
|
||||
// for the service lifetime to reuse the streaming dispatcher's connection pool.
|
||||
private readonly aiProviderFetch = createInstrumentedFetch(
|
||||
'AiService:provider-http',
|
||||
createStreamingFetch(),
|
||||
);
|
||||
@@ -152,13 +151,12 @@ export class AiService {
|
||||
// endpoint. The default callable createOpenAI(...)(model) targets the
|
||||
// Responses API (/responses), which OpenAI-compatible gateways
|
||||
// (OpenRouter, etc.) reject on multi-turn requests (history with
|
||||
// assistant messages) → 400.
|
||||
// DIAGNOSTIC (provider ECONNRESET investigation) — temporary: pass the
|
||||
// passive instrumented fetch (logging only; no behavior change).
|
||||
// assistant messages) → 400. The provider fetch is the instrumented
|
||||
// streaming fetch (finite-but-generous stream timeouts, #175).
|
||||
return createOpenAI({
|
||||
apiKey,
|
||||
baseURL: baseUrl,
|
||||
fetch: this.aiDiagnosticFetch,
|
||||
fetch: this.aiProviderFetch,
|
||||
}).chat(chatModel);
|
||||
case 'gemini':
|
||||
return createGoogleGenerativeAI({ apiKey })(chatModel);
|
||||
|
||||
Reference in New Issue
Block a user