HarperFast / HarperFast/symphony
idleTimeoutMs is a maximum connection duration, not an idle timeout
- Dominant language
- Rust
- Stars
- 1
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
Pre-existing on `main`, surfaced by the CI review bot on #33 (which only touched the error mapping around this code, not the timeout itself).
`proxy_conn.rs::forward` wraps the whole copy future in `tokio::time::timeout`:
```rust
match timeout(idle, body).await {
Ok(result) => result.map_err(|_| ErrorKind::Stream),
Err(_) => Err(ErrorKind::IdleTimeout),
}
```
`tokio::time::timeout` is a hard deadline — it does not reset on I/O activity. So `idleTimeoutMs` terminates any connection once its **total** duration exceeds the value, however busy it has been. A large file transfer, a long-lived MQTT subscription, or a streaming response all get cut at the deadline mid-flight.
The default is 60s, so out of the box any connection lasting longer than a minute is killed.
## Impact
host-manager already works around this without naming it — it sets `idleTimeoutMs: 0` on every listener, with a comment that 60s "would reap idle keep-alive sockets" and is below the lifetime of long-lived MQTT/replication connections. That disables the feature entirely rather than fixing it, so today there is effectively no idle timeout in the Fabric deployment.
Anyone who does set a non-zero `idleTimeoutMs` — following the README, which documents it as "Close connections silent for this many ms" — gets a duration cap instead.
## Fix sketch
A real idle timeout needs a deadline that resets on activity. Options:
- Wrap the streams in an adapter that bumps a shared `Instant` on every successful read/write, with a periodic checker task — composes with the existing `CountingStream`, which already sits on the client side of the copy.
- Or drive the copy manually with `tokio::select!` against a `tokio::time::Sleep` that gets `reset()` on each transfer.
Either way `ErrorKind::IdleTimeout` (added in #33 for the exported metrics) then means what its label says. Until this is fixed, that metric counts duration-cap terminations.
## Note
Not fixed in #33 deliberately — it is a behaviour change on the proxy's hottest path and unrelated to the metrics work. #33 documents the current semantics where the metric is defined.
Contributor guide
Research direction
Read proxy_conn.rs::forward and the README's idleTimeoutMs documentation first; compare the documented idle semantics with the timeout wrapping described here. Trace how CountingStream and ErrorKind::IdleTimeout participate in the copy path, then validate that a non-zero timeout resets on activity and that long active transfers are not terminated at the configured duration.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100