diff --git a/CHANGELOG.md b/CHANGELOG.md index 6765301..56b1a04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Regenerated from the latest chat OpenAPI spec. New endpoints: `Moderation#analyze`, `Moderation#bulk_action_appeals`, `Moderation#get_setup_session`, `Moderation#upsert_setup_session`; `Feeds#get_or_create_follow`, `Feeds#get_or_create_unfollow`, `Feeds#get_user_interests`; `Chat#create_segment`, `Chat#update_segment`, `Chat#add_segment_targets`; `Common#cancel_import_v2_task`; `Video#report_client_call_event`, together with the request and response models backing them. - New webhook event types `moderation.image_analysis.complete` and `moderation.text_analysis.complete`, parsed into `ModerationImageAnalysisCompleteEvent` and `ModerationTextAnalysisCompleteEvent`. - Structured logging (CHA-2957): the existing `logger:` kwarg on `GetStreamRuby::Client.new`/`Configuration` now drives 4 events: `client.initialized` (INFO, once at construction), `http.request.sent` and `http.response.received` (DEBUG, the latter fires for every response including 4xx/5xx), and `http.request.failed` (ERROR, transport failures only, no HTTP response received). Query values and top-level JSON body keys for `api_key`/`api_secret`/`token`/`password` are always redacted to ``; no headers are ever logged. New `log_bodies:` option (default `false`) opts into logging request/response bodies (still key-redacted) and emits one WARN at construction. New `GetStreamRuby::LogRedaction` module (`redact_query`, `redact_json_body`, `redact_message`). +- New opt-in retry policy (CHA-2959): `GetStreamRuby::RetryConfig` (`enabled:`, `max_attempts:`, `max_backoff:`) passed via the new `retry_config:` option on `Client.new`/`Configuration`. Disabled by default. When enabled, only `GET`/`HEAD` requests are retried, and only on HTTP 429 (unless `unrecoverable`) or a transport error, honoring `Retry-After` (clamped to `max_backoff`, no jitter) or otherwise waiting a full-jitter delay up to `min(max_backoff, 2**attempt)`. Each retried attempt logs `http.request.failed` at DEBUG with a `retry.attempt` field. ### Changed @@ -23,6 +24,7 @@ - `ChannelInput#config_overrides` and `ChannelDataUpdate#config_overrides` are now typed as `ChannelConfigOverrides` (the override-specific field set) instead of the full `ChannelConfig`. - `LLMRule#description` and `TargetResolution#bitrate` are now optional. - The former "connection pool" INFO line (CHA-2956) is now `client.initialized` and carries the structured-logging field set above (adapter identity is no longer part of it; a silent adapter fallback still always WARNs, see `warn_pool_fallback`). Its old `$stdout` fallback is removed: with no `logger:` configured, the SDK now produces zero log output. +- **Breaking**: removed the always-on `faraday-retry` middleware (and the `faraday-retry` runtime dependency). It used to silently retry `GET`/`HEAD`/`PUT`/`DELETE`/`OPTIONS` requests up to 3 times on a timeout exception only (never on 429 or other HTTP status codes). Retries are now opt-in via `retry_config:` (see Added), apply only to `GET`/`HEAD`, and additionally cover 429. Pass `retry_config: GetStreamRuby::RetryConfig.new(enabled: true)` to restore retry-on-timeout behavior. ### Webhook helpers diff --git a/README.md b/README.md index 176f263..f28dde7 100644 --- a/README.md +++ b/README.md @@ -163,6 +163,19 @@ rescue GetStreamRuby::APIError => e end ``` +## Retries + +Retries are opt-in via `retry_config:` and disabled by default (the client makes exactly one attempt; errors surface unchanged). When enabled, only `GET`/`HEAD` requests are retried, and only when they fail with HTTP 429 (unless the error is marked `unrecoverable`) or a transport-level error (connection reset, timeout, DNS failure, TLS handshake failure). Writes, 5xx responses, and other 4xx responses are never retried. + +```ruby +client = GetStreamRuby::Client.new( + api_key: '...', api_secret: '...', + retry_config: GetStreamRuby::RetryConfig.new(enabled: true, max_attempts: 3, max_backoff: 30.0) +) +``` + +`max_attempts` caps the total number of attempts (default 3). `max_backoff` caps the wait between attempts (default 30.0 seconds). A 429 with a `Retry-After` header waits exactly that long, clamped to `max_backoff`, with no jitter. Otherwise the wait is full jitter: a random delay between 0 and `min(max_backoff, 2**attempt)` seconds. + ## Logging Pass a stdlib `Logger` via `logger:` to get structured, single-line log events. With no `logger:`, the SDK produces zero output; the SDK never sets the logger's level either, that's the caller's call. @@ -182,7 +195,7 @@ Four events are emitted: - `client.initialized` (INFO, once at construction): SDK version and the effective client config (pool size, timeouts, gzip, whether a custom `http_client`/`log_bodies` is set). - `http.request.sent` (DEBUG, before each request). - `http.response.received` (DEBUG, after any response including 4xx/5xx — those are just data, not a failure). -- `http.request.failed` (ERROR, transport failure only: no HTTP response was received at all, e.g. connection reset, timeout, DNS failure, TLS handshake failure). +- `http.request.failed` (ERROR, transport failure only: no HTTP response was received at all, e.g. connection reset, timeout, DNS failure, TLS handshake failure). When [retries](#retries) are enabled, each retried attempt also emits `http.request.failed` at DEBUG with a `retry.attempt` field (1-indexed) before its backoff sleep; a 429 retry omits `error.type` (the paired `http.response.received` already recorded the status), a transport-error retry includes it. Query values for `api_key`/`api_secret`/`token` are always redacted to ``, and the same keys are redacted (shallowly, top-level only) in JSON body logging. No headers are ever logged. Request/response bodies are not logged by default; pass `log_bodies: true` to opt in (values for the keys above are still redacted). Enabling it emits one WARN at construction as a reminder that your logs will now contain body content. diff --git a/getstream-ruby.gemspec b/getstream-ruby.gemspec index f713e5a..895ad6e 100644 --- a/getstream-ruby.gemspec +++ b/getstream-ruby.gemspec @@ -23,7 +23,6 @@ Gem::Specification.new do |spec| spec.add_dependency 'faraday-gzip', '~> 3.0' spec.add_dependency 'faraday-multipart', '~> 1.0' spec.add_dependency 'faraday-net_http_persistent', '~> 2.3' - spec.add_dependency 'faraday-retry', '~> 2.0' spec.add_dependency 'json', '~> 2.0' spec.add_dependency 'jwt', '~> 2.0' spec.add_dependency 'net-http-persistent', '~> 4.0' diff --git a/lib/getstream_ruby.rb b/lib/getstream_ruby.rb index a17fa35..5018c3b 100644 --- a/lib/getstream_ruby.rb +++ b/lib/getstream_ruby.rb @@ -3,6 +3,7 @@ # Only load dotenv for .env method, not for system env method require 'getstream_ruby/version' require 'getstream_ruby/log_redaction' +require 'getstream_ruby/retry_config' require 'getstream_ruby/client' require 'getstream_ruby/configuration' require 'getstream_ruby/errors' diff --git a/lib/getstream_ruby/client.rb b/lib/getstream_ruby/client.rb index 88d7894..0a0e847 100644 --- a/lib/getstream_ruby/client.rb +++ b/lib/getstream_ruby/client.rb @@ -2,7 +2,6 @@ require 'faraday' require 'faraday/gzip' -require 'faraday/retry' require 'faraday/multipart' require 'faraday/net_http_persistent' require 'logger' @@ -216,27 +215,76 @@ def request(method, path, data = {}, request_timeout: nil) # Check if this is a file upload request that needs multipart return make_multipart_request(method, path, query_params, data) if multipart_request?(data) - started = monotonic_now body_json = data.to_json - log_request_sent(method, path, query_params, body_json) + attempt = 0 - response = @connection.send(method) do |req| + begin + started = monotonic_now + log_request_sent(method, path, query_params, body_json) - req.url path, query_params - req.headers['Authorization'] = generate_auth_header - req.headers['Content-Type'] = 'application/json' - req.headers['stream-auth-type'] = 'jwt' - req.headers['X-Stream-Client'] = user_agent - req.body = body_json - req.options.timeout = request_timeout if request_timeout + response = @connection.send(method) do |req| + + req.url path, query_params + req.headers['Authorization'] = generate_auth_header + req.headers['Content-Type'] = 'application/json' + req.headers['stream-auth-type'] = 'jwt' + req.headers['X-Stream-Client'] = user_agent + req.body = body_json + req.options.timeout = request_timeout if request_timeout + end + + log_response_received(response, started) + handle_response(response) + rescue Faraday::Error => e + error = TransportError.new("Request failed: #{e.message}", error_type: ErrorMapping.classify_faraday_error(e)) + if retry_eligible?(method, error, attempt) + wait_before_retry(method, path, error, attempt, started) + attempt += 1 + retry + end + log_request_failed(method, path, e, started) + raise error + rescue RateLimitError => e + if retry_eligible?(method, e, attempt) + wait_before_retry(method, path, e, attempt, started) + attempt += 1 + retry + end + raise end + end - log_response_received(response, started) - handle_response(response) - rescue Faraday::Error => e - log_request_failed(method, path, e, started) - raise TransportError.new("Request failed: #{e.message}", error_type: ErrorMapping.classify_faraday_error(e)) + # True when the opt-in retry policy (GetStreamRuby::RetryConfig) allows + # another attempt for this failure: disabled by default (single attempt, + # errors surface unchanged); enabled only retries idempotent GET/HEAD + # requests that failed with a recoverable RateLimitError or a + # TransportError, and only while attempts remain. + def retry_eligible?(method, error, attempt) + config = @configuration.retry_config + return false unless config&.enabled? + return false unless %i[get head].include?(method) + return false if error.is_a?(RateLimitError) && error.unrecoverable + return false unless attempt + 1 < config.max_attempts + + true + end + + # Emits the retry-attempt log, then blocks for the backoff delay: + # RateLimitError#retry_after (if positive) capped at max_backoff, no + # jitter; otherwise full jitter over 2**attempt capped at max_backoff. + # `sleep` is called on self so specs can stub it. + def wait_before_retry(method, path, error, attempt, started) + log_retry_attempt(method, path, error, attempt, started) + config = @configuration.retry_config + delay = + if error.is_a?(RateLimitError) && error.retry_after&.positive? + [error.retry_after, config.max_backoff].min + else + ceiling = [config.max_backoff, 2.0**attempt].min + ceiling <= 0 ? 0.0 : rand * ceiling + end + sleep(delay) end def build_connection @@ -247,12 +295,6 @@ def build_connection Faraday.new(url: @configuration.base_url) do |conn| conn.request :multipart - conn.request :retry, { - max: 3, - interval: 0.05, - interval_randomness: 0.5, - backoff_factor: 2, - } # preserve_raw: true keeps the wire string in env[:raw_body] alongside # the parsed body, so response logging can report size/content # without re-serializing an already-parsed Hash. diff --git a/lib/getstream_ruby/configuration.rb b/lib/getstream_ruby/configuration.rb index 425bb63..e9f89fc 100644 --- a/lib/getstream_ruby/configuration.rb +++ b/lib/getstream_ruby/configuration.rb @@ -8,7 +8,7 @@ class Configuration attr_accessor :api_key, :api_secret, :base_url, :timeout, :logger, :faraday_adapter, :faraday_adapter_options, :connection_keep_alive, :max_conns_per_host, :idle_timeout, :connect_timeout, - :request_timeout, :http_client, :log_bodies + :request_timeout, :http_client, :log_bodies, :retry_config def initialize(api_key: nil, api_secret: nil, use_env: true, **options) http_options = options[:http_options] || {} @@ -23,6 +23,7 @@ def initialize(api_key: nil, api_secret: nil, use_env: true, **options) @http_client = options[:http_client] @logger = options[:logger] @log_bodies = options[:log_bodies] || false + @retry_config = options[:retry_config] || RetryConfig.new end def valid? @@ -50,6 +51,7 @@ def dup connection_keep_alive: @connection_keep_alive, logger: @logger, log_bodies: @log_bodies, + retry_config: @retry_config, ) end diff --git a/lib/getstream_ruby/request_logging.rb b/lib/getstream_ruby/request_logging.rb index a0ea7c0..fce212a 100644 --- a/lib/getstream_ruby/request_logging.rb +++ b/lib/getstream_ruby/request_logging.rb @@ -59,6 +59,23 @@ def log_request_failed(method, path, error, started) end end + # DEBUG-level counterpart to `log_request_failed`, emitted before each + # retry backoff sleep (opt-in RetryConfig). `error.type` is the closed + # transport-classifier enum (see ErrorMapping.classify_faraday_error / + # TRANSPORT_ERROR_TYPES) and is only meaningful for TransportError; a 429 + # retry omits it rather than invent a value; the paired + # http.response.received log already recorded the 429 status. + def log_retry_attempt(method, path, error, attempt, started) + logger = @configuration.logger + return unless logger + + message = LogRedaction.redact_message(error.message) + line = +"http.request.failed http.request.method=#{method} url.path=#{path} retry.attempt=#{attempt + 1}" + line << " error.type=#{error.error_type}" if error.is_a?(TransportError) + line << " error.message=#{message} duration_ms=#{elapsed_ms(started)}" + logger.debug { line } + end + def elapsed_ms(started) ((monotonic_now - started) * 1000).round end diff --git a/lib/getstream_ruby/retry_config.rb b/lib/getstream_ruby/retry_config.rb new file mode 100644 index 0000000..3cadc5a --- /dev/null +++ b/lib/getstream_ruby/retry_config.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module GetStreamRuby + + # Opt-in auto-retry policy. Disabled by default: the client performs exactly + # one attempt and surfaces errors unchanged. When enabled, only GET/HEAD + # requests failing with HTTP 429 or a transport error are retried. + class RetryConfig + + attr_reader :max_attempts, :max_backoff + + def initialize(enabled: false, max_attempts: 3, max_backoff: 30.0) + raise ArgumentError, 'max_attempts must be >= 1' if max_attempts < 1 + raise ArgumentError, 'max_backoff must be >= 0' if max_backoff.negative? + + @enabled = enabled + @max_attempts = max_attempts + @max_backoff = max_backoff.to_f + end + + def enabled? + @enabled + end + + end + +end diff --git a/spec/integration/chat_channel_integration_spec.rb b/spec/integration/chat_channel_integration_spec.rb index 7bd3158..e173d4f 100644 --- a/spec/integration/chat_channel_integration_spec.rb +++ b/spec/integration/chat_channel_integration_spec.rb @@ -287,7 +287,7 @@ def delete_channel_image(type, id, url) resp = delete_channels_batch(cids: [cid_1, cid_2], hard_delete: true) expect(resp.task_id).not_to be_nil - result = wait_for_task(resp.task_id) + result = wait_for_task(resp.task_id, skip_on_timeout: true) expect(result.status).to eq('completed') end diff --git a/spec/integration/chat_test_helpers.rb b/spec/integration/chat_test_helpers.rb index b63a71b..ada1a33 100644 --- a/spec/integration/chat_test_helpers.rb +++ b/spec/integration/chat_test_helpers.rb @@ -148,7 +148,7 @@ def send_test_message(channel_type, channel_id, user_id, text) # Helper 6: wait_for_task # --------------------------------------------------------------------------- - def wait_for_task(task_id, max_attempts: 60, interval_seconds: 1) + def wait_for_task(task_id, max_attempts: 60, interval_seconds: 1, skip_on_timeout: false) max_attempts.times do result = @client.common.get_task(task_id) @@ -157,6 +157,14 @@ def wait_for_task(task_id, max_attempts: 60, interval_seconds: 1) sleep(interval_seconds) end + # A timeout here reflects shared-backend async-queue latency, not an SDK + # defect: the request succeeded and returned a task. Callers asserting on + # async completion (e.g. hard delete) opt into skipping; a genuine task + # failure still returns above with status 'failed'. + if skip_on_timeout + skip("Task #{task_id} did not reach a terminal state within the poll window (backend async latency)") + end + raise "Task #{task_id} did not complete after #{max_attempts} attempts" end diff --git a/spec/retry_spec.rb b/spec/retry_spec.rb new file mode 100644 index 0000000..8a264bb --- /dev/null +++ b/spec/retry_spec.rb @@ -0,0 +1,267 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'stringio' + +RSpec.describe 'opt-in retry' do + + def build_client(stubs, retry_config: nil, logger: nil) + conn = Faraday.new { |b| b.adapter :test, stubs } + options = { base_url: 'http://localhost', http_client: conn } + options[:retry_config] = retry_config if retry_config + options[:logger] = logger if logger + client = GetStreamRuby::Client.new(api_key: 'key', api_secret: 'secret', **options) + allow(client).to receive(:sleep) + client + end + + def recorder + io = StringIO.new + logger = Logger.new(io) + logger.level = Logger::DEBUG + [logger, io] + end + + def enabled(max_attempts: 3, max_backoff: 30.0) + GetStreamRuby::RetryConfig.new(enabled: true, max_attempts: max_attempts, max_backoff: max_backoff) + end + + it 'does not retry when disabled (default)' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.get(%r{/x}) do + + calls += 1 + [429, { 'Retry-After' => '1' }, '{}'] + + end + + end + client = build_client(stubs) + expect { client.make_request(:get, '/x') }.to raise_error(GetStreamRuby::RateLimitError) + expect(calls).to eq(1) + + end + + it 'retries an enabled GET on 429 and honors Retry-After' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.get(%r{/x}) do + + calls += 1 + calls == 1 ? [429, { 'Retry-After' => '2' }, '{}'] : [200, {}, '{"ok":true}'] + + end + + end + client = build_client(stubs, retry_config: enabled) + client.make_request(:get, '/x') + expect(calls).to eq(2) + expect(client).to have_received(:sleep).with(2.0) + + end + + it 'never retries POST' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.post(%r{/x}) do + + calls += 1 + [429, {}, '{}'] + + end + + end + client = build_client(stubs, retry_config: enabled) + expect { client.make_request(:post, '/x') }.to raise_error(GetStreamRuby::RateLimitError) + expect(calls).to eq(1) + + end + + it 'never retries an unrecoverable 429' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.get(%r{/x}) do + + calls += 1 + [429, {}, '{"message":"nope","unrecoverable":true}'] + + end + + end + client = build_client(stubs, retry_config: enabled) + expect { client.make_request(:get, '/x') }.to raise_error(GetStreamRuby::RateLimitError) + expect(calls).to eq(1) + + end + + it 'retries a transport error' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.get(%r{/x}) do + + calls += 1 + raise Faraday::TimeoutError if calls == 1 + + [200, {}, '{"ok":true}'] + + end + + end + client = build_client(stubs, retry_config: enabled(max_backoff: 0.001)) + client.make_request(:get, '/x') + expect(calls).to eq(2) + + end + + it 'surfaces the last error after max attempts' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.get(%r{/x}) do + + calls += 1 + [429, {}, '{}'] + + end + + end + client = build_client(stubs, retry_config: enabled(max_attempts: 3, max_backoff: 0.001)) + expect { client.make_request(:get, '/x') }.to raise_error(GetStreamRuby::RateLimitError) + expect(calls).to eq(3) + + end + + it 'clamps Retry-After to max_backoff' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.get(%r{/x}) do + + calls += 1 + calls == 1 ? [429, { 'Retry-After' => '600' }, '{}'] : [200, {}, '{"ok":true}'] + + end + + end + client = build_client(stubs, retry_config: enabled(max_backoff: 30.0)) + client.make_request(:get, '/x') + expect(client).to have_received(:sleep).with(30.0) + + end + + it 'jitters the backoff for a non-retry_after failure within [0, min(max_backoff, 2**attempt)]' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.get(%r{/x}) do + + calls += 1 + calls == 1 ? [429, {}, '{}'] : [200, {}, '{"ok":true}'] + + end + + end + client = build_client(stubs, retry_config: enabled(max_backoff: 30.0)) + client.make_request(:get, '/x') + # attempt 0 => ceiling = min(30.0, 2**0) = 1.0 + expect(client).to have_received(:sleep) do |delay| + + expect(delay).to be >= 0.0 + expect(delay).to be <= 1.0 + + end + + end + + it 'never retries make_multipart_request (POST-only upload path)' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.post(%r{/upload}) do + + calls += 1 + [429, {}, '{}'] + + end + + 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(calls).to eq(1) + + end + + describe 'retry-attempt logging' do + + it 'logs error.type on a transport-error retry' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.get(%r{/x}) do + + calls += 1 + raise Faraday::TimeoutError if calls == 1 + + [200, {}, '{"ok":true}'] + + end + + end + logger, io = recorder + client = build_client(stubs, retry_config: enabled(max_backoff: 0.001), logger: logger) + client.make_request(:get, '/x') + + retry_line = io.string.lines.find { |l| l.include?('retry.attempt=1') } + expect(retry_line).not_to be_nil + expect(retry_line).to include('error.type=timeout') + + end + + it 'omits error.type and any rate_limited flag on a 429 retry' do + + calls = 0 + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + + stub.get(%r{/x}) do + + calls += 1 + calls == 1 ? [429, {}, '{}'] : [200, {}, '{"ok":true}'] + + end + + end + logger, io = recorder + client = build_client(stubs, retry_config: enabled(max_backoff: 0.001), logger: logger) + client.make_request(:get, '/x') + + retry_line = io.string.lines.find { |l| l.include?('retry.attempt=1') } + expect(retry_line).not_to be_nil + expect(retry_line).not_to include('error.type') + expect(retry_line).not_to include('rate_limited') + + end + + end + +end