Skip to content

Ensure connection is closed for single-value SSE streams#264

Merged
rsamoilov merged 1 commit intorage-rb:mainfrom
jsxs0:fix-single-value-sse-connection-leak
Apr 7, 2026
Merged

Ensure connection is closed for single-value SSE streams#264
rsamoilov merged 1 commit intorage-rb:mainfrom
jsxs0:fix-single-value-sse-connection-leak

Conversation

@jsxs0
Copy link
Copy Markdown
Contributor

@jsxs0 jsxs0 commented Apr 7, 2026

Summary

Add ensure connection.close to SSE::Application#send_data so the Iodine connection is always closed for single-value SSE streams, even when Rage::SSE.__serialize or connection.write raises.

Problem

start_formatted_stream (Enumerator path) has ensure connection.close. start_raw_stream (Proc path) has a rescue block (#248). But send_data (single-value path) has no error handling — if __serialize raises (e.g. to_json on an object with circular references, encoding errors, or a broken to_json implementation), connection.close never executes and the connection leaks until Iodine's socket timeout.

# Before — connection.close only runs if serialize + write succeed
def send_data(connection)
  Rage::Telemetry.tracer.span_sse_stream_process(connection:, type: @type) do
    connection.write(Rage::SSE.__serialize(@stream))
    connection.close
  end
end

# After — connection always closes, matching start_formatted_stream pattern
def send_data(connection)
  Rage::Telemetry.tracer.span_sse_stream_process(connection:, type: @type) do
    connection.write(Rage::SSE.__serialize(@stream))
  end
ensure
  connection.close
end

Why ensure (not rescue)?

Single-value streams are synchronous and one-shot — the connection should always close after send_data, regardless of success or failure. This matches the ensure connection.close pattern already used by start_formatted_stream.

Test plan

Added 3 test cases for #send_data (single-value streams had no dedicated tests):

  • Single value → writes serialized data and closes connection
  • Serialization raises → connection still closed
  • Write raises → connection still closed
bundle exec rspec spec/sse/ — 53 examples, 0 failures

@jsxs0 jsxs0 force-pushed the fix-single-value-sse-connection-leak branch from 3a53e13 to 1b486de Compare April 7, 2026 06:55
@jsxs0 jsxs0 force-pushed the fix-single-value-sse-connection-leak branch from 1b486de to 60388fc Compare April 7, 2026 06:58
Copy link
Copy Markdown
Member

@rsamoilov rsamoilov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thank you 🚀

I'm extremely happy you're looking into the SSE functionality given your experience!

@rsamoilov rsamoilov merged commit a560c1b into rage-rb:main Apr 7, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants