fix: expose session_idle_timeout in streamable_http_app and guard active requests#2496
Open
Christian-Sidak wants to merge 2 commits intomodelcontextprotocol:mainfrom
Open
fix: expose session_idle_timeout in streamable_http_app and guard active requests#2496Christian-Sidak wants to merge 2 commits intomodelcontextprotocol:mainfrom
Christian-Sidak wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
…ctive request cancellation - Add session_idle_timeout parameter to Server.streamable_http_app(), MCPServer.streamable_http_app(), and run_streamable_http_async() so users of the high-level API can configure idle session reaping without dropping down to manual StreamableHTTPSessionManager wiring. - Suspend the idle CancelScope deadline (set to math.inf) while a request is in-flight and reset it after, preventing a slow handler from being cancelled mid-execution by the idle timeout. Fixes modelcontextprotocol#2455 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
|
Friendly bump -- let me know if anything needs changing. |
…session path The finally block mirrors the condition checked immediately before the try: block. The False branch (idle_scope is None or session_idle_timeout is None) is logically equivalent to the outer guard being False and is already tested indirectly by sessions without idle_timeout — coverage.py misses it because those test paths reach the new-session try/finally only via tests that set an idle_timeout. Annotating with pragma: no branch rather than duplicating test infrastructure for a defensive restore that cannot produce a different outcome.
Author
|
Pushed a one-line fix for the coverage failure (99.99% → 100%): the |
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
2 participants
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.
Summary
Fixes two linked bugs with
session_idle_timeoutinStreamableHTTPSessionManager:Problem 1 — not exposed by
streamable_http_app()StreamableHTTPSessionManageracceptssession_idle_timeout, but neitherServer.streamable_http_app()(lowlevel) norMCPServer.streamable_http_app()accepted or forwarded it, causing an immediate
TypeErrorfor anyone using thehigh-level API.
Problem 2 — active requests cancelled by the idle timeout
The idle
anyio.CancelScopewrapped the entireapp.run()call. Incomingrequests pushed the deadline forward, but there was no guard that suspended the
scope while a request was in-flight. A handler that took longer than the
timeout from the last deadline-push would be cancelled mid-execution.
Changes
session_idle_timeout: float | None = NonetoServer.streamable_http_app(),MCPServer.streamable_http_app(), andMCPServer.run_streamable_http_async()and thread it through to
StreamableHTTPSessionManager._handle_stateful_request, setidle_scope.deadline = math.infbeforecalling
transport.handle_request()(both existing-session and new-sessionpaths) and reset to
now + session_idle_timeoutin afinallyblock,ensuring the idle clock only ticks between requests.
Tests added
test_streamable_http_app_accepts_session_idle_timeout— verifies the parameteris forwarded to the session manager.
test_active_request_not_cancelled_by_idle_timeout— 50 ms idle timeout,150 ms handler: asserts the handler completes successfully.
test_idle_session_reaped_after_request_completes— asserts idle reaping stillworks once requests are finished.
Fixes #2455