[v1.x] Drop responses/notifications when write stream is already closed#2502
[v1.x] Drop responses/notifications when write stream is already closed#2502bobbyo wants to merge 2 commits into
Conversation
|
We hit this in production with mcp 1.28.1 / FastMCP on StreamableHTTP stateless mode: 18 occurrences across 4 workspaces over 2026-07-19–20, always the same stack: The trigger: an This PR (#2502) directly addresses it by hardening Happy to help with review, testing, or rebasing if that would unblock this. |
|
Friendly bump on this one 🙂 Wanted to share a quick status check in case it's helpful for review prioritization:
Given the production impact we're seeing (18 crashes across 4 workspaces in 2 days on 1.28.1, per the report above), we'd really appreciate a maintainer look when you have a moment. Happy to help with anything — additional test coverage, a rebase, or answering questions — if that would help move it along. Thanks so much for the work on this fix! 🙏 |
Audience / Scope / Status
BaseSession, not only in outer server layerspyrightfailures on the first pushLast verified against:
v1.xon 2026-04-24Relevant upstream issues
ClosedResourceErrorafter client disconnect. Relevant because the failing write path here is also in the shared session layer.Summary
This PR makes
BaseSession.send_notification()andBaseSession._send_response()treat writes to an already-closed transport as expected no-ops rather than exceptions.The goal is narrow: if the peer has already gone away, a late response or notification should be dropped cleanly at the session send boundary.
Failure topology
Why the catch belongs here
mcp.server.lowlevel.serverawait message.respond(response)atsrc/mcp/server/lowlevel/server.py:799-809RequestResponderpath that reaches this exact outer frameBaseSession.send_notification()_write_stream.send(...)ClosedResourceError/BrokenResourceErrorBaseSession._send_response()_write_stream.send(...)Invariants
Changes
anyio.BrokenResourceError/anyio.ClosedResourceErrorinBaseSession.send_notification()atsrc/mcp/shared/session.py:318-339.anyio.BrokenResourceError/anyio.ClosedResourceErrorinBaseSession._send_response()atsrc/mcp/shared/session.py:343-357.tests/server/test_session.py:223-282.tests/server/test_lowlevel_exception_handling.py:78-96andsrc/mcp/server/lowlevel/server.py:799-809.Validation
Functional
PYTHONPATH=src python3 -m pytest -o addopts='' \ tests/server/test_session.py \ tests/server/test_lowlevel_exception_handling.py -qLocal result after the CI-fix follow-up:
17 passedCI follow-up
The first push exposed two CI-only problems, not a runtime regression:
testmatrixlowlevel/server.pyunexercisedtest_handle_request_drops_response_when_transport_is_closed()to cover the existing outer branchpre-commitpyrightrejected loosely typed fake session/write-stream helperstests/server/test_session.pyTargeted local follow-up verification:
Local result:
0 errors, 0 warnings, 0 informationsNon-goals
lowlevel/server.py; it keeps defense in depth.