Skip to content

Commit 9c68789

Browse files
committed
Use rescue StandardError for stream close
## Motivation and Context Replace bare `rescue` in stream cleanup with `rescue StandardError` so signals and exits are not swallowed. ## How Has This Been Tested? Add regression tests for `cleanup_session_unsafe` and `reap_expired_sessions`. ## Breaking Changes None.
1 parent 5631990 commit 9c68789

File tree

2 files changed

+56
-4
lines changed

2 files changed

+56
-4
lines changed

lib/mcp/server/transports/streamable_http_transport.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ def reap_expired_sessions
150150
# removed from `@sessions` above, so other threads will not find them
151151
# and will not attempt to close the same stream.
152152
stream.close
153-
rescue
154-
nil
153+
rescue StandardError
154+
# Ignore close-related errors from already closed/broken streams.
155155
end
156156
end
157157

@@ -239,8 +239,8 @@ def cleanup_session_unsafe(session_id)
239239

240240
begin
241241
session[:stream]&.close
242-
rescue
243-
nil
242+
rescue StandardError
243+
# Ignore close-related errors from already closed/broken streams.
244244
end
245245
@sessions.delete(session_id)
246246
end

test/mcp/server/transports/streamable_http_transport_test.rb

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,58 @@ class StreamableHTTPTransportTest < ActiveSupport::TestCase
579579
assert_equal({}, @transport.instance_variable_get(:@sessions))
580580
end
581581

582+
test "reap_expired_sessions rescues StandardError from stream close" do
583+
transport = StreamableHTTPTransport.new(@server, session_idle_timeout: 0.01)
584+
585+
init_request = create_rack_request(
586+
"POST",
587+
"/",
588+
{ "CONTENT_TYPE" => "application/json" },
589+
{ jsonrpc: "2.0", method: "initialize", id: "init" }.to_json,
590+
)
591+
init_response = transport.handle_request(init_request)
592+
session_id = init_response[1]["Mcp-Session-Id"]
593+
594+
# Replace the stream with one that raises `IOError` on close.
595+
error_stream = Object.new
596+
error_stream.define_singleton_method(:close) { raise IOError, "already closed" }
597+
transport.instance_variable_get(:@sessions)[session_id][:stream] = error_stream
598+
599+
sleep(0.02) # Wait for session to expire.
600+
601+
# Should not raise because `StandardError` is rescued.
602+
transport.send(:reap_expired_sessions)
603+
assert_empty(transport.instance_variable_get(:@sessions))
604+
ensure
605+
transport.close
606+
end
607+
608+
test "cleanup_session_unsafe rescues StandardError from stream close" do
609+
init_request = create_rack_request(
610+
"POST",
611+
"/",
612+
{ "CONTENT_TYPE" => "application/json" },
613+
{ jsonrpc: "2.0", method: "initialize", id: "init" }.to_json,
614+
)
615+
init_response = @transport.handle_request(init_request)
616+
session_id = init_response[1]["Mcp-Session-Id"]
617+
618+
# Replace the stream with one that raises `IOError` on close.
619+
error_stream = Object.new
620+
error_stream.define_singleton_method(:close) { raise IOError, "already closed" }
621+
@transport.instance_variable_get(:@sessions)[session_id][:stream] = error_stream
622+
623+
# Should not raise because `StandardError` is rescued.
624+
delete_request = create_rack_request(
625+
"DELETE",
626+
"/",
627+
{ "HTTP_MCP_SESSION_ID" => session_id },
628+
)
629+
response = @transport.handle_request(delete_request)
630+
assert_equal 200, response[0]
631+
assert_empty @transport.instance_variable_get(:@sessions)
632+
end
633+
582634
test "sends notification to correct session with multiple active sessions" do
583635
# Create first session
584636
init_request1 = create_rack_request(

0 commit comments

Comments
 (0)