fix: enforce timeouts.total_seconds as an aggregate request deadline - #169
Open
shrdgn wants to merge 1 commit into
Open
fix: enforce timeouts.total_seconds as an aggregate request deadline#169shrdgn wants to merge 1 commit into
shrdgn wants to merge 1 commit into
Conversation
total_seconds has been documented and set in every examples/*.yaml.example since it was added, but nothing ever enforced it -- only member_seconds and judge_seconds bounded individual calls (issue #140). Non-streaming fused/pipeline requests now wrap buffer_synthesis/buffer_vote/ buffer_ranked/buffer_pipeline in asyncio.wait_for(timeout=total_seconds), raising a clean RequestTimeoutError (504) on expiry. Streaming requests pull each SSE line from the underlying generator the same way in _stream_with_cancellation, so a single stuck upstream call gets cut off, not just the gaps between chunks -- matching how member_seconds/judge_seconds already bound calls. A deadline timeout emits one SSE error chunk + [DONE] (the client is still there, unlike on disconnect, where cancellation stays silent). Closes #140.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
timeouts.total_secondshas been documented and set in everyexamples/*.yaml.examplesince it was added, but nothing ever enforced it —only
member_secondsandjudge_secondsbounded individual upstream calls(#140).
buffer_synthesis/buffer_vote/buffer_ranked/buffer_pipeline) are now wrapped inasyncio.wait_for(timeout=total_seconds), raising a cleanRequestTimeoutError(504,timeout_error/total_timeout) on expiry._stream_with_cancellation, shared by_pipeline_streamand
_fusion_stream) now pull each SSE line from the underlying generatorthe same way (
asyncio.wait_forper__anext__()), so a single stuckupstream call gets cut off — not just the gaps between already-yielded
chunks — matching how
member_seconds/judge_secondsalready bound calls.A deadline timeout emits one SSE
errorchunk +[DONE](the client isstill connected, unlike on disconnect, where cancellation stays silent).
TimeoutsConfig's docstring now documentstotal_secondsas an enforcedaggregate cap instead of leaving it as an undocumented, unused field.
Closes #140.
How it was tested
ruff check .passespytest -qpasses (497 passed, no live network) —mypy openfusion/also passes
tests/test_total_timeout.py(4 end-to-endtests: non-streaming/streaming × fusion/pipeline, each against a mocked
upstream that never returns, verifying the request is cut off at
total_secondsrather than hanging) andtests/test_stream_cancellation.py(unit tests for
_stream_with_cancellation's new deadline branch,including the disconnect-vs-deadline distinction and the loop-top
already-elapsed edge case)
TimeoutsConfigdocstring,CHANGELOG.mdNotes for reviewers
total_secondsis documented andvalidated — see fix: reject configs where timeouts.total_seconds can't cover member+judge #151 — specifically against
member_seconds + judge_seconds); pass-through requests are untouched.gen.__anext__()call inasyncio.wait_forrather than relying solely on the existing cooperativecancel_event(which only interrupts a streamer between its owncheckpoints). This mirrors the non-streaming path's semantics — a
single hung upstream call is what
total_secondsis meant to catch, perits own docs/examples.
_stream_with_cancellation'sfinallynow also callsgen.aclose()so anearly cutoff (deadline or disconnect) runs any cleanup in the underlying
streamer rather than just abandoning it.
Generated by Claude Code