From 5a4af5884b183682d5d2b7aadfa328c15ab1b373 Mon Sep 17 00:00:00 2001 From: Aditya Agarwal Date: Fri, 28 Aug 2026 19:06:59 +0200 Subject: [PATCH 1/2] fix: retry once when a pooled keep-alive socket is already dead [CHA-4943] Faraday/net_http_persistent reuses dead sockets and surfaces SSL EOF, RST, or ReadTimeout-on-closed-socket as TransportError. Match Go net/http by retrying unused idle connections once with no backoff, including POST. DNS failures and real read timeouts are not retried. Co-authored-by: Cursor --- CHANGELOG.md | 6 ++ lib/getstream_ruby/client.rb | 7 +- lib/getstream_ruby/error_mapping.rb | 37 +++++++- spec/errors_spec.rb | 24 +++++ spec/retry_spec.rb | 139 +++++++++++++++++++++++++++- 5 files changed, 206 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71ecf64..7932b44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ### Fixed +- Retry a request **once** when the pooled keep-alive socket is already dead + (`SSL_read` EOF, `Connection reset by peer`, `TCPSocket:(closed)`). Matches + Go `net/http` (retry unused idle connections). Applies to POST as well as + GET; DNS failures and real read timeouts are not retried. No backoff; the + opt-in `retry_config:` policy is unchanged. + - Default `idle_timeout` for `net_http_persistent` is now `25` seconds (was `55`). GCP SSL-proxy load balancers close idle TLS around ~30s; keeping pooled connections for 55s (AWS ALB idle minus 5s) caused diff --git a/lib/getstream_ruby/client.rb b/lib/getstream_ruby/client.rb index d262c54..bc11b4a 100644 --- a/lib/getstream_ruby/client.rb +++ b/lib/getstream_ruby/client.rb @@ -216,7 +216,7 @@ def request(method, path, data = {}, request_timeout: nil) return make_multipart_request(method, path, query_params, data) if multipart_request?(data) body_json = data.to_json - attempt = 0 + attempt = stale_retries = 0 begin started = monotonic_now @@ -238,6 +238,11 @@ def request(method, path, data = {}, request_timeout: nil) handle_response(response) rescue Faraday::Error => e error = TransportError.new("Request failed: #{e.message}", error_type: ErrorMapping.classify_faraday_error(e)) + if stale_retries.zero? && ErrorMapping.stale_keep_alive?(e) + log_retry_attempt(method, path, error, stale_retries, started) + stale_retries += 1 + retry + end if retry_eligible?(method, error, attempt) wait_before_retry(method, path, error, attempt, started) attempt += 1 diff --git a/lib/getstream_ruby/error_mapping.rb b/lib/getstream_ruby/error_mapping.rb index bf70f43..6257570 100644 --- a/lib/getstream_ruby/error_mapping.rb +++ b/lib/getstream_ruby/error_mapping.rb @@ -10,6 +10,17 @@ module ErrorMapping module_function + STALE_KEEP_ALIVE_PATTERN = / + connection\ reset\ by\ peer + |unexpected\ eof + |ssl_read + |tcpsocket:\(closed\) + |broken\ pipe + |connection\ is\ closed + |end\ of\ file\ reached + |tls_retry_write_records + /ix.freeze + # Raises the appropriate `ApiError` / `RateLimitError` for a non-2xx # `Faraday::Response`. def raise_api_error(response) @@ -111,7 +122,7 @@ def classify_faraday_error(error) end def classify_connection_failure(error) - wrapped = error.respond_to?(:wrapped_exception) ? error.wrapped_exception : nil + wrapped = wrapped_exception(error) case wrapped when SocketError 'dns_failure' @@ -120,6 +131,30 @@ def classify_connection_failure(error) end end + # True when the failure looks like a reused keep-alive socket that the peer + # already closed (Go net/http `errServerClosedIdle` / `nothingWrittenError`). + # DNS failures and real read timeouts are not stale-pool errors. + def stale_keep_alive?(error) + return false if error.nil? + return false if classify_faraday_error(error) == 'dns_failure' + return true if error.is_a?(Faraday::SSLError) || error.is_a?(Faraday::ConnectionFailed) + + stale_keep_alive_message?(error) + end + + def wrapped_exception(error) + return nil unless error.respond_to?(:wrapped_exception) + + error.wrapped_exception + end + + def stale_keep_alive_message?(error) + texts = [error.message] + wrapped = wrapped_exception(error) + texts << wrapped.message if wrapped.respond_to?(:message) + texts.compact.any? { |text| text.match?(STALE_KEEP_ALIVE_PATTERN) } + end + def build_task_error(task_id, error_payload) hash = if error_payload.respond_to?(:to_h) error_payload.to_h diff --git a/spec/errors_spec.rb b/spec/errors_spec.rb index e8ccacb..f47d085 100644 --- a/spec/errors_spec.rb +++ b/spec/errors_spec.rb @@ -316,6 +316,30 @@ end + describe 'ErrorMapping.stale_keep_alive?' do + + it 'treats SSL EOF and connection reset as stale keep-alive' do + + ssl = Faraday::SSLError.new('SSL_read: unexpected eof while reading') + reset = Faraday::ConnectionFailed.new('Connection reset by peer') + closed = Faraday::TimeoutError.new('Net::ReadTimeout with #') + expect(GetStreamRuby::ErrorMapping.stale_keep_alive?(ssl)).to be(true) + expect(GetStreamRuby::ErrorMapping.stale_keep_alive?(reset)).to be(true) + expect(GetStreamRuby::ErrorMapping.stale_keep_alive?(closed)).to be(true) + + end + + it 'does not treat a real timeout or DNS failure as stale keep-alive' do + + timeout = Faraday::TimeoutError.new('Net::ReadTimeout') + dns = Faraday::ConnectionFailed.new(SocketError.new('getaddrinfo failed')) + expect(GetStreamRuby::ErrorMapping.stale_keep_alive?(timeout)).to be(false) + expect(GetStreamRuby::ErrorMapping.stale_keep_alive?(dns)).to be(false) + + end + + end + describe 'transport-layer failures' do it 'wraps Faraday::ConnectionFailed as TransportError with cause preserved' do diff --git a/spec/retry_spec.rb b/spec/retry_spec.rb index 8a264bb..9b0d4c6 100644 --- a/spec/retry_spec.rb +++ b/spec/retry_spec.rb @@ -84,6 +84,138 @@ def enabled(max_attempts: 3, max_backoff: 30.0) end + describe 'stale keep-alive retry' do + + it 'retries POST once on Connection reset by peer without retry_config' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.post(%r{/x}) do + + calls += 1 + raise Faraday::ConnectionFailed, 'Connection reset by peer' if calls == 1 + + [200, {}, '{"ok":true}'] + + end + + end + client = build_client(stubs) + client.make_request(:post, '/x') + expect(calls).to eq(2) + expect(client).not_to have_received(:sleep) + + end + + it 'retries POST once on SSL_read unexpected eof' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.post(%r{/x}) do + + calls += 1 + raise Faraday::SSLError, 'SSL_read: unexpected eof while reading' if calls == 1 + + [200, {}, '{"ok":true}'] + + end + + end + client = build_client(stubs) + client.make_request(:post, '/x') + expect(calls).to eq(2) + + end + + it 'retries POST once on ReadTimeout with a closed TCPSocket' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.post(%r{/x}) do + + calls += 1 + raise Faraday::TimeoutError, 'Net::ReadTimeout with #' if calls == 1 + + [200, {}, '{"ok":true}'] + + end + + end + client = build_client(stubs) + client.make_request(:post, '/x') + expect(calls).to eq(2) + + end + + it 'does not retry POST on a real read timeout' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.post(%r{/x}) do + + calls += 1 + raise Faraday::TimeoutError, 'Net::ReadTimeout' + + end + + end + client = build_client(stubs) + expect { client.make_request(:post, '/x') }.to raise_error(GetStreamRuby::TransportError) + expect(calls).to eq(1) + + end + + it 'does not retry a DNS failure' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.post(%r{/x}) do + + calls += 1 + # Faraday needs the SocketError as wrapped_exception for DNS classification. + raise Faraday::ConnectionFailed.new( # rubocop:disable Style/RaiseArgs + SocketError.new('getaddrinfo: nodename nor servname provided'), + ) + + end + + end + client = build_client(stubs) + expect { client.make_request(:post, '/x') }.to raise_error(GetStreamRuby::TransportError) do |err| + + expect(err.error_type).to eq('dns_failure') + + end + expect(calls).to eq(1) + + end + + it 'retries a stale connection only once' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.post(%r{/x}) do + + calls += 1 + raise Faraday::ConnectionFailed, 'Connection reset by peer' + + end + + end + client = build_client(stubs) + expect { client.make_request(:post, '/x') }.to raise_error(GetStreamRuby::TransportError) + expect(calls).to eq(2) + + end + + end + it 'never retries an unrecoverable 429' do calls = 0 @@ -202,11 +334,8 @@ def enabled(max_attempts: 3, max_backoff: 30.0) end client = build_client(stubs, retry_config: enabled) file = GetStream::Generated::Models::FileUploadRequest.new(file: __FILE__) - expect do - - client.send(:make_multipart_request, :post, '/upload', {}, file) - - end.to raise_error(GetStreamRuby::RateLimitError) + expect { client.send(:make_multipart_request, :post, '/upload', {}, file) } + .to raise_error(GetStreamRuby::RateLimitError) expect(calls).to eq(1) end From f888533604505ca4b3cbbfe1315b4e52b35490fe Mon Sep 17 00:00:00 2001 From: Aditya Agarwal Date: Mon, 31 Aug 2026 14:31:07 +0200 Subject: [PATCH 2/2] fix: match stale keep-alive by error text, not Faraday class [CHA-4943] SSLError and ConnectionFailed also cover cert failures and connection refused. Retry once only when the message says the pooled socket is already dead. Co-authored-by: Cursor --- CHANGELOG.md | 9 ++++--- lib/getstream_ruby/error_mapping.rb | 5 ++-- spec/errors_spec.rb | 11 +++++++++ spec/retry_spec.rb | 38 +++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7932b44..990b139 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,10 +3,11 @@ ### Fixed - Retry a request **once** when the pooled keep-alive socket is already dead - (`SSL_read` EOF, `Connection reset by peer`, `TCPSocket:(closed)`). Matches - Go `net/http` (retry unused idle connections). Applies to POST as well as - GET; DNS failures and real read timeouts are not retried. No backoff; the - opt-in `retry_config:` policy is unchanged. + (`unexpected eof`, `Connection reset by peer`, `TCPSocket:(closed)`). + Detected from the error message, not Faraday class (so cert failures and + connection refused are not retried). Applies to POST as well as GET; DNS + failures and real read timeouts are not retried. No backoff; the opt-in + `retry_config:` policy is unchanged. - Default `idle_timeout` for `net_http_persistent` is now `25` seconds (was `55`). GCP SSL-proxy load balancers close idle TLS around ~30s; keeping pooled diff --git a/lib/getstream_ruby/error_mapping.rb b/lib/getstream_ruby/error_mapping.rb index 6257570..0013592 100644 --- a/lib/getstream_ruby/error_mapping.rb +++ b/lib/getstream_ruby/error_mapping.rb @@ -13,7 +13,6 @@ module ErrorMapping STALE_KEEP_ALIVE_PATTERN = / connection\ reset\ by\ peer |unexpected\ eof - |ssl_read |tcpsocket:\(closed\) |broken\ pipe |connection\ is\ closed @@ -132,12 +131,12 @@ def classify_connection_failure(error) end # True when the failure looks like a reused keep-alive socket that the peer - # already closed (Go net/http `errServerClosedIdle` / `nothingWrittenError`). + # already closed. Match the error text, not Faraday class: SSLError and + # ConnectionFailed also cover cert failures and connection refused. # DNS failures and real read timeouts are not stale-pool errors. def stale_keep_alive?(error) return false if error.nil? return false if classify_faraday_error(error) == 'dns_failure' - return true if error.is_a?(Faraday::SSLError) || error.is_a?(Faraday::ConnectionFailed) stale_keep_alive_message?(error) end diff --git a/spec/errors_spec.rb b/spec/errors_spec.rb index f47d085..878710c 100644 --- a/spec/errors_spec.rb +++ b/spec/errors_spec.rb @@ -338,6 +338,17 @@ end + it 'does not treat connection refused or cert failures as stale keep-alive' do + + refused = Faraday::ConnectionFailed.new('Connection refused') + cert = Faraday::SSLError.new('certificate verify failed') + ssl_read = Faraday::SSLError.new('SSL_read: wrong version number') + expect(GetStreamRuby::ErrorMapping.stale_keep_alive?(refused)).to be(false) + expect(GetStreamRuby::ErrorMapping.stale_keep_alive?(cert)).to be(false) + expect(GetStreamRuby::ErrorMapping.stale_keep_alive?(ssl_read)).to be(false) + + end + end describe 'transport-layer failures' do diff --git a/spec/retry_spec.rb b/spec/retry_spec.rb index 9b0d4c6..8d00ae1 100644 --- a/spec/retry_spec.rb +++ b/spec/retry_spec.rb @@ -169,6 +169,44 @@ def enabled(max_attempts: 3, max_backoff: 30.0) end + it 'does not retry POST on connection refused' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.post(%r{/x}) do + + calls += 1 + raise Faraday::ConnectionFailed, 'Connection refused' + + end + + end + client = build_client(stubs) + expect { client.make_request(:post, '/x') }.to raise_error(GetStreamRuby::TransportError) + expect(calls).to eq(1) + + end + + it 'does not retry POST on an SSL certificate failure' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.post(%r{/x}) do + + calls += 1 + raise Faraday::SSLError, 'certificate verify failed' + + end + + end + client = build_client(stubs) + expect { client.make_request(:post, '/x') }.to raise_error(GetStreamRuby::TransportError) + expect(calls).to eq(1) + + end + it 'does not retry a DNS failure' do calls = 0