From 14bf349350d99e4f9921c2d738f84b8f1f505fbe Mon Sep 17 00:00:00 2001 From: jsxs0 <44266373+jsxs0@users.noreply.github.com> Date: Thu, 26 Mar 2026 18:06:14 +0900 Subject: [PATCH 1/2] Ensure connection is closed when raw SSE stream raises Add an `ensure` block to `start_raw_stream` so the connection is always closed if the user's Proc raises an exception without calling `close`. This matches the cleanup behavior already present in `start_formatted_stream`, which has `ensure connection.close`. Without this fix, a Proc that raises before closing leaves the Iodine connection open until socket timeout. --- lib/rage/sse/application.rb | 2 ++ spec/sse/application_spec.rb | 68 ++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 spec/sse/application_spec.rb diff --git a/lib/rage/sse/application.rb b/lib/rage/sse/application.rb index 19b3e555..2ad70649 100644 --- a/lib/rage/sse/application.rb +++ b/lib/rage/sse/application.rb @@ -54,5 +54,7 @@ def start_formatted_stream(connection) def start_raw_stream(connection) @stream.call(Rage::SSE::ConnectionProxy.new(connection)) + ensure + connection.close if connection.open? end end diff --git a/spec/sse/application_spec.rb b/spec/sse/application_spec.rb new file mode 100644 index 00000000..e067ffe8 --- /dev/null +++ b/spec/sse/application_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +RSpec.describe Rage::SSE::Application do + let(:connection) { MockSSEConnection.new } + + class MockSSEConnection + attr_reader :messages + + def initialize + @messages = [] + @open = true + end + + def write(data) + @messages << data + end + + def close + @open = false + end + + def open? + @open + end + end + + describe "#start_raw_stream" do + it "closes the connection when the proc raises an exception" do + failing_proc = ->(conn) { + conn.write("data: before error\n\n") + raise "boom" + } + + app = described_class.new(failing_proc) + + expect { + app.send(:start_raw_stream, connection) + }.to raise_error(RuntimeError, "boom") + + expect(connection.open?).to be false + end + + it "does not double-close if the proc already closed the connection" do + well_behaved_proc = ->(conn) { + conn.write("data: hello\n\n") + conn.close + } + + app = described_class.new(well_behaved_proc) + app.send(:start_raw_stream, connection) + + expect(connection.open?).to be false + expect(connection.messages).to eq(["data: hello\n\n"]) + end + + it "closes the connection on normal completion even if proc forgets to close" do + forgetful_proc = ->(conn) { + conn.write("data: forgot to close\n\n") + # User forgot to call conn.close + } + + app = described_class.new(forgetful_proc) + app.send(:start_raw_stream, connection) + + expect(connection.open?).to be false + end + end +end From 819f747db12af72eb1dd88d8e6268967d140556a Mon Sep 17 00:00:00 2001 From: jsxs0 <44266373+jsxs0@users.noreply.github.com> Date: Sat, 28 Mar 2026 13:36:28 +0900 Subject: [PATCH 2/2] Close connection only on exception, not on normal completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback: the ensure block breaks async patterns where the proc spawns background fibers and returns early (e.g. Datastar SDK). Changed to rescue so the connection is only closed when the proc raises, not when it completes normally. Updated tests: - Proc raises → connection closed - Proc returns without closing (async pattern) → connection stays open - Proc closes itself → no interference --- lib/rage/sse/application.rb | 3 ++- spec/sse/application_spec.rb | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/rage/sse/application.rb b/lib/rage/sse/application.rb index 2ad70649..2aa1d057 100644 --- a/lib/rage/sse/application.rb +++ b/lib/rage/sse/application.rb @@ -54,7 +54,8 @@ def start_formatted_stream(connection) def start_raw_stream(connection) @stream.call(Rage::SSE::ConnectionProxy.new(connection)) - ensure + rescue => e connection.close if connection.open? + raise e end end diff --git a/spec/sse/application_spec.rb b/spec/sse/application_spec.rb index e067ffe8..7c08d31e 100644 --- a/spec/sse/application_spec.rb +++ b/spec/sse/application_spec.rb @@ -40,29 +40,29 @@ def open? expect(connection.open?).to be false end - it "does not double-close if the proc already closed the connection" do - well_behaved_proc = ->(conn) { - conn.write("data: hello\n\n") - conn.close + it "does not close the connection on normal completion" do + async_proc = ->(conn) { + conn.write("data: started\n\n") + # Proc returns without closing — a background fiber will close later } - app = described_class.new(well_behaved_proc) + app = described_class.new(async_proc) app.send(:start_raw_stream, connection) - expect(connection.open?).to be false - expect(connection.messages).to eq(["data: hello\n\n"]) + expect(connection.open?).to be true end - it "closes the connection on normal completion even if proc forgets to close" do - forgetful_proc = ->(conn) { - conn.write("data: forgot to close\n\n") - # User forgot to call conn.close + it "does not interfere when the proc closes the connection itself" do + well_behaved_proc = ->(conn) { + conn.write("data: hello\n\n") + conn.close } - app = described_class.new(forgetful_proc) + app = described_class.new(well_behaved_proc) app.send(:start_raw_stream, connection) expect(connection.open?).to be false + expect(connection.messages).to eq(["data: hello\n\n"]) end end end