From b2c5f111af55b4ba6971d21333c2ccc56d6e526c Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 24 Mar 2026 13:16:01 -0400 Subject: [PATCH 1/6] feat: Expand version support for http.rb v6 This commit adds support for http.rb v6 while maintaining backward compatibility with v4 and v5. The http.rb v6 release changed from accepting an options hash to requiring keyword arguments for the HTTP::Client constructor and related methods. Key changes: - Update gemspec to allow http.rb versions up to 7.0.0 - Add VALID_HTTP_CLIENT_OPTIONS constant to filter options passed to HTTP::Client, preventing errors from unsupported options - Convert all HTTP client option keys from strings to symbols - Update HTTP::Client initialization and method calls to use keyword arguments (**options) instead of hash arguments - Add EOFError handling for improved connection cleanup in v6 - Add comprehensive test coverage for options filtering The filtering mechanism ensures that only valid HTTP client options are passed through, which is particularly important for maintaining backward compatibility when custom http_client_options are provided. --- ld-eventsource.gemspec | 2 +- lib/ld-eventsource/client.rb | 31 ++++++++---- spec/client_spec.rb | 97 ++++++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 10 deletions(-) diff --git a/ld-eventsource.gemspec b/ld-eventsource.gemspec index ed6f5c2..348c4ed 100644 --- a/ld-eventsource.gemspec +++ b/ld-eventsource.gemspec @@ -28,5 +28,5 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'webrick', '~> 1.7' spec.add_runtime_dependency 'concurrent-ruby', '~> 1.0' - spec.add_runtime_dependency 'http', '>= 4.4.1', '< 6.0.0' + spec.add_runtime_dependency 'http', '>= 4.4.1', '< 7.0.0' end diff --git a/lib/ld-eventsource/client.rb b/lib/ld-eventsource/client.rb index ea5f1bf..1291697 100644 --- a/lib/ld-eventsource/client.rb +++ b/lib/ld-eventsource/client.rb @@ -52,6 +52,15 @@ class Client # The default HTTP method for requests. DEFAULT_HTTP_METHOD = "GET" + # TODO(breaking): Remove this filtering once we have updated to the next major breaking version. + # HTTP v6 requires keyword arguments instead of an options hash, so we filter to only known valid + # arguments to avoid passing unsupported options. + VALID_HTTP_CLIENT_OPTIONS = %i[ + base_uri body encoding features follow form headers json keep_alive_timeout + nodelay params persistent proxy response retriable socket_class ssl_context + ssl ssl_socket_class timeout_class timeout_options + ].freeze + # # Creates a new SSE client. # @@ -126,7 +135,7 @@ def initialize(uri, base_http_client_options = {} if socket_factory - base_http_client_options["socket_class"] = socket_factory + base_http_client_options[:socket_class] = socket_factory end if proxy @@ -139,22 +148,24 @@ def initialize(uri, end if @proxy - base_http_client_options["proxy"] = { + base_http_client_options[:proxy] = { :proxy_address => @proxy.host, :proxy_port => @proxy.port, } - base_http_client_options["proxy"][:proxy_username] = @proxy.user unless @proxy.user.nil? - base_http_client_options["proxy"][:proxy_password] = @proxy.password unless @proxy.password.nil? + base_http_client_options[:proxy][:proxy_username] = @proxy.user unless @proxy.user.nil? + base_http_client_options[:proxy][:proxy_password] = @proxy.password unless @proxy.password.nil? end options = http_client_options.is_a?(Hash) ? base_http_client_options.merge(http_client_options) : base_http_client_options + options = options.transform_keys(&:to_sym) + options = options.select { |key, _| VALID_HTTP_CLIENT_OPTIONS.include?(key) } - @http_client = HTTP::Client.new(options) + @http_client = HTTP::Client.new(**options) .follow - .timeout({ + .timeout( read: read_timeout, connect: connect_timeout, - }) + ) @cxn = nil @lock = Mutex.new @@ -342,7 +353,7 @@ def connect begin uri = build_uri_with_query_params @logger.info { "Connecting to event stream at #{uri}" } - cxn = @http_client.request(@method, uri, build_opts) + cxn = @http_client.request(@method, uri, **build_opts) headers = cxn.headers if cxn.status.code == 200 content_type = cxn.content_type.mime_type @@ -390,8 +401,10 @@ def read_stream(cxn) rescue HTTP::TimeoutError # For historical reasons, we rethrow this as our own type raise Errors::ReadTimeoutError.new(@read_timeout) + rescue EOFError + break end - break if data.nil? + break if data.nil? # keep for v5 compat gen.yield data end end diff --git a/spec/client_spec.rb b/spec/client_spec.rb index 7a8bfe4..4c502ad 100644 --- a/spec/client_spec.rb +++ b/spec/client_spec.rb @@ -908,6 +908,103 @@ def test_object.to_s end end + describe "http_client_options filtering" do + it "filters out unsupported options" do + with_server do |server| + server.setup_response("/") do |req,res| + send_stream_content(res, "", keep_open: true) + end + + client = subject.new(server.base_uri, + http_client_options: { + "socket_class" => "MySocket", + "ssl" => { verify_mode: 0 }, + "not_a_real_option" => "should be removed", + "another_fake" => 123, + }) + + http_client = client.instance_variable_get(:@http_client) + options = http_client.default_options + + expect(options.socket_class).to eq("MySocket") + expect(options.ssl).to eq({ verify_mode: 0 }) + + client.close + end + end + + it "filters out unsupported options provided as symbols" do + with_server do |server| + server.setup_response("/") do |req,res| + send_stream_content(res, "", keep_open: true) + end + + client = subject.new(server.base_uri, + http_client_options: { + socket_class: "MySocket", + not_a_real_option: "should be removed", + }) + + http_client = client.instance_variable_get(:@http_client) + options = http_client.default_options + + expect(options.socket_class).to eq("MySocket") + + client.close + end + end + + it "does not raise when only unsupported options are provided" do + with_server do |server| + server.setup_response("/") do |req,res| + send_stream_content(res, "", keep_open: true) + end + + client = nil + expect { + client = subject.new(server.base_uri, + http_client_options: { + "totally_fake" => true, + "also_fake" => "yes", + }) + }.not_to raise_error + + client.close + end + end + + it "preserves all valid options" do + with_server do |server| + server.setup_response("/") do |req,res| + send_stream_content(res, "", keep_open: true) + end + + socket_factory = double("SocketFactory") + ssl_socket_factory = double("SSLSocketFactory") + + client = subject.new(server.base_uri, + http_client_options: { + socket_class: socket_factory, + ssl_socket_class: ssl_socket_factory, + nodelay: true, + keep_alive_timeout: 30, + ssl: { verify_mode: 0 }, + }) + + http_client = client.instance_variable_get(:@http_client) + options = http_client.default_options + + expect(options.socket_class).to eq(socket_factory) + expect(options.ssl_socket_class).to eq(ssl_socket_factory) + expect(options.nodelay).to eq(true) + expect(options.keep_alive_timeout).to eq(30) + expect(options.ssl).to eq({ verify_mode: 0 }) + + client.close + end + end + end + describe "retry parameter" do it "defaults to true (retries enabled)" do events_body = simple_event_1_text From daa821629cf54c03f9f2dd17c048364ae51d35b4 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 24 Mar 2026 15:41:27 -0400 Subject: [PATCH 2/6] log on dropped option --- lib/ld-eventsource/client.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/ld-eventsource/client.rb b/lib/ld-eventsource/client.rb index 1291697..d04351e 100644 --- a/lib/ld-eventsource/client.rb +++ b/lib/ld-eventsource/client.rb @@ -158,7 +158,11 @@ def initialize(uri, options = http_client_options.is_a?(Hash) ? base_http_client_options.merge(http_client_options) : base_http_client_options options = options.transform_keys(&:to_sym) - options = options.select { |key, _| VALID_HTTP_CLIENT_OPTIONS.include?(key) } + options = options.select do |key, _| + included = VALID_HTTP_CLIENT_OPTIONS.include?(key) + @logger.warn { "Ignoring unsupported HTTP client option: #{key}" } unless included + included + end @http_client = HTTP::Client.new(**options) .follow From 9218011f94c4abb35e8ece8480bafe2c0e46586d Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 24 Mar 2026 15:44:51 -0400 Subject: [PATCH 3/6] rubocop fix --- lib/ld-eventsource/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ld-eventsource/client.rb b/lib/ld-eventsource/client.rb index d04351e..68926e9 100644 --- a/lib/ld-eventsource/client.rb +++ b/lib/ld-eventsource/client.rb @@ -168,7 +168,7 @@ def initialize(uri, .follow .timeout( read: read_timeout, - connect: connect_timeout, + connect: connect_timeout ) @cxn = nil @lock = Mutex.new From 74b006e6b955cac3e2b7af920815c1decd52272a Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 24 Mar 2026 15:58:43 -0400 Subject: [PATCH 4/6] only include timeout options if set --- lib/ld-eventsource/client.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/ld-eventsource/client.rb b/lib/ld-eventsource/client.rb index 68926e9..5954be6 100644 --- a/lib/ld-eventsource/client.rb +++ b/lib/ld-eventsource/client.rb @@ -127,8 +127,8 @@ def initialize(uri, @retry_enabled = retry_enabled @headers = headers.clone - @connect_timeout = connect_timeout - @read_timeout = read_timeout + @connect_timeout = connect_timeout&.to_f + @read_timeout = read_timeout&.to_f @method = method.to_s.upcase @payload = payload @logger = logger || default_logger @@ -164,12 +164,13 @@ def initialize(uri, included end + timeout_options = {} + timeout_options[:connect_timeout] = @connect_timeout if @connect_timeout + timeout_options[:read_timeout] = @read_timeout if @read_timeout + @http_client = HTTP::Client.new(**options) .follow - .timeout( - read: read_timeout, - connect: connect_timeout - ) + .timeout(timeout_options) @cxn = nil @lock = Mutex.new From 9462484b378d058eb22e0810a46835368e5ebc24 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 24 Mar 2026 16:53:50 -0400 Subject: [PATCH 5/6] use short form --- lib/ld-eventsource/client.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ld-eventsource/client.rb b/lib/ld-eventsource/client.rb index 5954be6..b4cae58 100644 --- a/lib/ld-eventsource/client.rb +++ b/lib/ld-eventsource/client.rb @@ -165,8 +165,8 @@ def initialize(uri, end timeout_options = {} - timeout_options[:connect_timeout] = @connect_timeout if @connect_timeout - timeout_options[:read_timeout] = @read_timeout if @read_timeout + timeout_options[:connect] = @connect_timeout if @connect_timeout + timeout_options[:read] = @read_timeout if @read_timeout @http_client = HTTP::Client.new(**options) .follow From 00aa2b002935cd272815c11721b902e4a67203b2 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 24 Mar 2026 17:11:04 -0400 Subject: [PATCH 6/6] guard against empty options --- lib/ld-eventsource/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ld-eventsource/client.rb b/lib/ld-eventsource/client.rb index b4cae58..bf6dbc8 100644 --- a/lib/ld-eventsource/client.rb +++ b/lib/ld-eventsource/client.rb @@ -170,7 +170,7 @@ def initialize(uri, @http_client = HTTP::Client.new(**options) .follow - .timeout(timeout_options) + @http_client = @http_client.timeout(timeout_options) unless timeout_options.empty? @cxn = nil @lock = Mutex.new