fix(streaming): default read timeout to 10s to match the terminal#1077
Merged
Conversation
The streaming read timeout default was 3s while the terminal's streaming client uses a 10s socket read timeout before treating a silent connection as dead and reconnecting. Right after subscribing to a full-market stream the server can go a few seconds fully silent (no data, no heartbeat) while it sets up the subscription; the 3s deadline trips inside that gap and forces an unnecessary reconnect. The 10s deadline rides through it. The knob stays configurable via streaming.timeout_ms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Aligns the library’s default streaming “no frames received” read timeout with the terminal client to avoid unnecessary reconnects during the initial post-subscribe quiet period, while updating binding-surface docs/tests to reflect the new default and correct liveness-signal descriptions.
Changes:
- Bumps the production default
streaming.timeout_msfrom 3,000ms to 10,000ms in the Rust core and updates pinned tests. - Sweeps the new default through generated/accessor docs across TS, Python, FFI/C, and C++ surfaces.
- Updates sample config + changelog entry to document the new default and rationale.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| thetadatadx-ts/src/_generated/config_accessors.rs | Updates TS N-API accessor docs to reflect 10s default. |
| thetadatadx-ts/index.d.ts | Updates TypeScript declaration docs for the 10s default. |
| thetadatadx-rs/src/config/mod.rs | Updates production-defaults test expectation for streaming.timeout_ms. |
| thetadatadx-rs/src/config/fpss.rs | Updates streaming config docs and production default to 10s; adjusts related tests. |
| thetadatadx-rs/config.default.toml | Updates sample config comments and read_timeout default to 10s. |
| thetadatadx-rs/config_surface.toml | Updates the doc-source-of-truth for binding surfaces to 10s. |
| thetadatadx-py/tests/test_config_resilience.py | Updates Python resilience test defaults/round-trip values to match 10s default. |
| thetadatadx-py/src/_generated/config_accessors.rs | Updates generated Python accessor docs to 10s default. |
| thetadatadx-py/python/thetadatadx/init.pyi | Updates Python type stub docstring default to 10s. |
| thetadatadx-ffi/src/config_accessors.rs | Updates C-ABI accessor docs to 10s default. |
| thetadatadx-ffi/src/auth.rs | Updates FFI resilience knob test default expectation to 10s. |
| thetadatadx-cpp/include/thetadatadx.h | Updates C header docs for the 10s default. |
| thetadatadx-cpp/include/config_accessors.hpp.inc | Updates C++ accessor docs for the 10s default. |
| CHANGELOG.md | Notes the default timeout change and rationale in the release notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+43
| # dead (ms). Matches the terminal's streaming socket read timeout and rides | ||
| # through the brief silence right after a full-market subscribe while the | ||
| # server sets the subscription up. The ~100 ms cadence is the client's ping | ||
| # to the server, not an inbound heartbeat; inbound frames arrive ~250 ms. | ||
| read_timeout = 10000 |
Comment on lines
+137
to
+141
| /// [`crate::RemoveReason::TimedOut`]. Default `10_000`, matching the | ||
| /// terminal's streaming socket read timeout. Right after a | ||
| /// full-market subscribe the server can fall fully silent (no | ||
| /// frames, no pings) for a few seconds while it sets the | ||
| /// subscription up; a 10 s deadline rides through that gap where a |
The ping_interval_ms docs described a server-side ~100ms heartbeat as the primary reverse-direction liveness signal, contradicting the timeout_ms docs. ping_interval_ms is the client's outbound ping (default 250ms); reverse-direction liveness is the inbound frame and ping stream (~250ms) that timeout_ms guards. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The TS config-resilience test still pinned the streaming read timeout default at 3_000n and round-tripped through 10_000n; both now reflect the 10s default, with the setter round-trip moved to 15_000n to stay discriminating. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The connection module header still documented a 3 second read-timeout default; the production default is 10 seconds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| host_selection: HostSelectionPolicy::Shuffled, | ||
| host_shuffle_seed: None, | ||
| timeout_ms: 3_000, | ||
| timeout_ms: 10_000, |
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.
The streaming client declares a session dead and reconnects when the socket read deadline (streaming.timeout_ms) elapses with no frames. That default was 3s while the terminal's streaming client uses a 10s socket read timeout.
On a real session, right after subscribing to a full-market stream the server can go about 3s fully silent (no data, no heartbeat) while it sets up the subscription. The 3s deadline trips inside that gap and forces an unnecessary reconnect, logging
FPSS read timed out timeout_ms=3000 quiet_ms=~3015. The 10s deadline rides through it. Verified live twice.This sets the production default to 10s to match the terminal exactly. The knob stays configurable via
streaming.timeout_ms(validated range unchanged at[100, 60_000]ms).Also corrects a stale doc claim on this knob: the ~100ms cadence is the client's ping to the server, not an inbound heartbeat; inbound frames and pings arrive roughly every ~250ms on an active session.
Swept the default across every binding surface: Rust core default and its pinned tests, the FFI/C-ABI/C++/Python/TS accessor docs (regenerated from
config_surface.toml), the sampleconfig.default.toml, and the Python type stub.🤖 Generated with Claude Code