Uh oh!
There was an error while loading. Please reload this page.
fix: add init_timeout for streamable-http sessions - #811
Conversation
3ec3a2a to
ddd3da7Compare| "get initialize request", | ||
| ) | ||
| })?; | ||
| let init_timeout = self.session_config.init_timeout.unwrap_or(Duration::MAX); |
There was a problem hiding this comment.
I don't understand why the _or(Duration::MAX) here. Aren't we setting a default init timeout of 60s?
There was a problem hiding this comment.
@alexhancock You're right. The default is 60s but users can set it to None to disable the timeout. unwrap_or(Duration::MAX) handles that opt-out case. This mirrors how keep_alive is handled a few lines below.
There was a problem hiding this comment.
Right, but I was thinking we should not have
pub init_timeout: Option<Duration>
pub init_timeout: Duration
seems more clear to me in this situation where it has a builtin default that is always set. It would make it so if the user wants to make it really long they could do the Duration::MAX
Uh oh!
There was an error while loading. Please reload this page.
Brings in upstream rmcp v1.6.0: - fix(http): fall back to :authority for HTTP/2 (modelcontextprotocol#827) - fix: add init_timeout for streamable-http sessions (modelcontextprotocol#811) - feat(http): log Host/Origin rejections (modelcontextprotocol#826) Conflict resolution: - crates/rmcp/CHANGELOG.md: kept fork's bare-boolean Unreleased entry, inserted upstream's 1.6.0 release section beneath it - crates/rmcp/src/transport/streamable_http_server/tower.rs: auto-merged upstream's Host/Origin/HTTP-2 logging additions; kept fork's tracing::debug! for "Resume failed" (ab4ccdb) over upstream's tracing::warn! revert - Cargo.toml workspace bumped to 1.6.0; fork's newer dep versions (pastey 0.2.2, schemars 1.2, reqwest 0.13.3, url 2.5, process-wrap 9.1, chrono 0.4.44) preserved; rand stays removed (ed5868d) since fork doesn't use it - docs.rs anthropic-ext feature retained
Fixes#808.
Motivation and Context
The
LocalSessionWorker::run()function waited for the firstinitializerequest using justself.event_rx.recv().awaitwithout a timeout. Any HTTP POST that created a session but didn’t follow up could keep a worker task running indefinitely. The only way to free it was through a server shutdown, which eventually led to memory exhaustion.The post-init loop already had a
keep_alivetimeout in place, but there was an issue with the pre-init path that created an asymmetric gap.How Has This Been Tested?
Added two integration tests
Breaking Changes
No API breaks. Both
SessionConfigandLocalSessionWorkerErrorare#[non_exhaustive].Types of changes
Checklist
Additional context