fix: stop applying the 45s SSE open timeout to non-streaming chat requests - #4743
fix: stop applying the 45s SSE open timeout to non-streaming chat requests#4743vibecoding-skills wants to merge 1 commit into
Conversation
…uests The bounded stream-open wait (DEFAULT_STREAM_OPEN_TIMEOUT, 45s) was wrapped around create_message_chat — the non-streaming path — where response headers only arrive after the server finishes the ENTIRE generation. Any non-streaming completion slower than 45s (e.g. a local model on Ollama with a real-world prompt) was aborted mid-generation with a misleading SSE error, while handle_chat_completion_stream — the SSE path the timeout was written for, per its own doc comment — had no open timeout at all and could still hang forever on the Windows/proxy paths described there. - create_message_chat now uses a non-streaming response budget: default 600s, overridable via CODEWHALE_NONSTREAM_TIMEOUT_SECS (clamped 30..3600), with an error message that explains the wait. - handle_chat_completion_stream now applies stream_open_timeout() to the header wait, as originally intended. - Adds a clamp unit test mirroring the existing stream_open_timeout one. Repro: codewhale --provider ollama --model <any local model> exec '<prompt long enough that generation exceeds 45s>' -> 'SSE stream request did not receive response headers after 45s' and Ollama logs a canceled request.
|
Thanks @vibecoding-skills for taking the time to contribute. This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered. Please read |
|
Thank you for this one, @vibecoding-skills — the analysis was correct, and it deserves a straight answer about why it is closing without merging. Your report was accurate when you filed it. Both halves are now fixed on
So Worth being explicit: an earlier triage pass of ours had this queued as an open bug to harvest. Re-checking against |
Each of these people had a PR triaged during the v0.9.2 pass, and three of them had work reach main without a Co-authored-by trailer because they were not in this file. Adding them so future harvests attribute correctly and the check-coauthor-trailers gate can resolve them. - EvanProgramming — #4760; equivalent change landed as 0d73127 with no trailer. - adity982 — #4756; the MCP call-once fix landed as cccaa4f with no trailer. - vibecoding-skills — #4743; the SSE-timeout split landed as a374d19. - XhesicaFrost — #4610, pending harvest for #4520. - ffaacceelee — #4686, closed without harvest, mapped pre-emptively. Numeric noreply ids verified against the GitHub users API rather than inferred, since a wrong id silently drops credit out of the contributor graph instead of failing loudly.
Problem
codewhale exec(plain, non---auto) against any non-streaming backend aborts with a misleading error whenever generation takes longer than 45 seconds:This makes plain
execeffectively unusable with local models (Ollama, llama-server, vLLM without streaming) on any real-world prompt: short prompts finish under the wire, but e.g. a "rewrite this 300-line file" task is killed mid-generation. On the server side Ollama logs a canceled request (error reading llama-server chat response: context canceled).Reproduction
Packet capture shows the request is delivered in full immediately; the client then closes the connection 45s later while the server is still generating.
DEEPSEEK_FORCE_HTTP1=1(which the error suggests) does not help — the transport is fine.Cause
The bounded header wait was attached to the wrong path in
crates/tui/src/client/chat.rs:create_message_chat— the non-streaming path (no"stream": truein the body) — wrapssend_json_with_retryintokio_timeout(stream_open_timeout(), …). But a non-streaming server only returns response headers after the entire generation completes, so the 45s "connection setup" budget races the model's full thinking + generation time. The doc comment onstream_open_timeoutitself says the timeout "only covers connection setup and upstream header return, not model thinking time after streaming has started" — which is only true when streaming is actually requested.handle_chat_completion_stream— the SSE path the timeout was written for — had no open timeout at all, so the Windows/proxy header hang described in that comment was still unbounded there.Fix
create_message_chatnow uses a non-streaming response budget: default 600s, overridable viaCODEWHALE_NONSTREAM_TIMEOUT_SECS(clamped 30..3600), with an error message that explains that non-streaming servers legitimately hold the response for the whole generation.handle_chat_completion_streamnow appliesstream_open_timeout()to its header wait, as originally intended (existing 45s default andCODEWHALE_STREAM_OPEN_TIMEOUT_SECSoverride unchanged).stream_open_timeout_defaults_and_clamps_env_values.Testing
cargo check -p codewhale-tuicleancargo fmtapplied;cargo clippy -p codewhale-tui --bin codewhale-tuicleancargo test -p codewhale-tui --bin codewhale-tui timeout— 53 passed, 0 failed (includes the new test)execinvocation for 2m42s and exits 0.