Skip to content

fix: enforce timeouts.total_seconds as an aggregate request deadline - #169

Open
shrdgn wants to merge 1 commit into
mainfrom
claude/vibrant-archimedes-r7u8ah
Open

fix: enforce timeouts.total_seconds as an aggregate request deadline#169
shrdgn wants to merge 1 commit into
mainfrom
claude/vibrant-archimedes-r7u8ah

Conversation

@shrdgn

@shrdgn shrdgn commented Aug 30, 2026

Copy link
Copy Markdown
Owner

What & why

timeouts.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 upstream calls
(#140).

  • Non-streaming fused/pipeline requests (buffer_synthesis/buffer_vote/
    buffer_ranked/buffer_pipeline) are now wrapped in
    asyncio.wait_for(timeout=total_seconds), raising a clean
    RequestTimeoutError (504, timeout_error/total_timeout) on expiry.
  • Streaming requests (_stream_with_cancellation, shared by _pipeline_stream
    and _fusion_stream) now pull each SSE line from the underlying generator
    the same way (asyncio.wait_for per __anext__()), so a single stuck
    upstream call
    gets cut off — not just the gaps between already-yielded
    chunks — matching how member_seconds/judge_seconds already bound calls.
    A deadline timeout emits one SSE error chunk + [DONE] (the client is
    still connected, unlike on disconnect, where cancellation stays silent).
  • TimeoutsConfig's docstring now documents total_seconds as an enforced
    aggregate cap instead of leaving it as an undocumented, unused field.

Closes #140.

How it was tested

  • ruff check . passes
  • pytest -q passes (497 passed, no live network) — mypy openfusion/
    also passes
  • New behavior has a test — tests/test_total_timeout.py (4 end-to-end
    tests: non-streaming/streaming × fusion/pipeline, each against a mocked
    upstream that never returns, verifying the request is cut off at
    total_seconds rather than hanging) and tests/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)
  • Docs updated — TimeoutsConfig docstring, CHANGELOG.md
  • N/A — no prompts/response bodies touched
  • N/A — no quality/cost claims

Notes for reviewers

  • Scope: only the fused/pipeline paths (total_seconds is documented and
    validated — 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.
  • Design choice: streaming enforcement wraps each gen.__anext__() call in
    asyncio.wait_for rather than relying solely on the existing cooperative
    cancel_event (which only interrupts a streamer between its own
    checkpoints). This mirrors the non-streaming path's semantics — a
    single hung upstream call is what total_seconds is meant to catch, per
    its own docs/examples.
  • _stream_with_cancellation's finally now also calls gen.aclose() so an
    early cutoff (deadline or disconnect) runs any cleanup in the underlying
    streamer rather than just abandoning it.

Generated by Claude Code

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

timeouts.total_seconds is documented and set in every example config, but never enforced

2 participants