Skip to content

fix: Keep a stream open for as long as it runs, rather than end it after five minutes. - #269

Merged
goloroden merged 3 commits into
mainfrom
fix/no-total-timeout-for-streams
Oct 3, 2026
Merged

goloroden merged 3 commits into
mainfrom
fix/no-total-timeout-for-streams

Conversation

@goloroden

Copy link
Copy Markdown
Member

HttpClient created its aiohttp.ClientSession without a timeout, so aiohttp used its default ClientTimeout(total=300, sock_connect=30), and total also covers reading the response body. Every observe_events stream therefore ended after exactly five minutes with a bare TimeoutError, even while the server sent a heartbeat every second, and so did any other stream that legitimately ran longer (a long run_eventql_query, read_events over a big store).

  • The five calls that read a stream of NDJSON lines (read_events, run_eventql_query, observe_events, read_subjects, read_event_types) pass is_stream=True to HttpClient.post, which gives that request ClientTimeout(total=None, sock_connect=30): no limit on how long it runs, while connecting still fails after 30 seconds. How long a stream may run is up to the caller (for example with asyncio.timeout); a stalled observe stream or query ends after 30 seconds without a line through the heartbeat timeout (feat: End a stream with an error when neither an event nor a heartbeat arrives for 30 seconds. #268).
  • Every other request (ping, write_events, verify_api_token, register_event_schema, read_event_type) keeps the session's limit, unchanged. It is per request because aiohttp reads a per-request timeout=None as no limit at all, connecting included. The public Client API is unchanged; HttpClient.post only gains an optional argument.
  • README: one sentence at the top of "Aborting Observing".
  • The local test server from feat: End a stream with an error when neither an event nor a heartbeat arrives for 30 seconds. #268 now answers every POST path, so the new tests use it, too.

Tests (with aiohttp's default patched to a 1 s total): observing against the real EventSourcingDB container stays open for the test's full 3 s deadline; a query against the local server sending heartbeats does the same; a write to a server that never sends its body still ends through the session limit, before the deadline. Red first: both stream tests ended after 1.01 s / 1.00 s through the session limit. A deliberately over-broad fix (no total for the whole session) fails the write guard. make qa green (lock check, ruff, pyright, bandit, 92 tests).

🤖 Generated with Claude Code

https://claude.ai/code/session_01TCdPte6nY6ToUVd2prb9GM

goloroden and others added 3 commits October 3, 2026 23:47
…ter five minutes.

The aiohttp session used aiohttp's default timeout, whose total of five
minutes also covers reading the response body. Every stream therefore ended
with a TimeoutError after five minutes, even while the server kept sending
heartbeats. Requests that read a stream (read events, run an EventQL query,
observe events, read subjects, read event types) now have no total limit,
only the 30 seconds for connecting. All other requests keep the limit of
the session.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TCdPte6nY6ToUVd2prb9GM
…of the stream timeout need no server of their own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TCdPte6nY6ToUVd2prb9GM
@goloroden
goloroden requested a review from a team as a code owner October 3, 2026 21:52
@goloroden goloroden self-assigned this Oct 3, 2026
@goloroden
goloroden merged commit c608357 into main Oct 3, 2026
4 checks passed
@goloroden
goloroden deleted the fix/no-total-timeout-for-streams branch October 3, 2026 21:54
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.

1 participant