diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a8aa52..496e29d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- Support configuring `Faraday::ConnectionFailed` and `Faraday::TimeoutError` + retry behaviour independently via `Service.new(connection_failed_retry_options:, + timeout_retry_options:)`, defaulting connection failures to a more generous + retry budget than timeouts + ### Changed +- `Faraday::ResourceNotFound` (404) is no longer retried, since a 404 is not a + transient failure and retrying it only adds latency + - Lowered minimum supported Ruby version and removed the pinned `.ruby-version` file in favor of explicit versions per CI workflow - Added a CI test matrix covering Ruby 3.4 and 4.0 diff --git a/README.md b/README.md index f7cd62b..668dc93 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,20 @@ all optional except `url`: - `logger` - an object responding to the standard `Logger` levels (`info`, `warn`, `error`, `debug`), used to log request/response details. Defaults to `Rails.logger` when running under Rails, otherwise `nil` +- `connection_failed_retry_options` - a hash of + [`faraday-retry`](https://github.com/lostisland/faraday-retry) options + (`max`, `interval`, `interval_randomness`, `backoff_factor`) applied to + `Faraday::ConnectionFailed` errors (e.g. a colocated service restarting). + Merged over the default of `max: 4, interval: 0.5, interval_randomness: 0.25, + backoff_factor: 2` +- `timeout_retry_options` - same shape as above, applied to + `Faraday::TimeoutError` errors. Merged over the default of `max: 2, + interval: 0.25, interval_randomness: 0.5, backoff_factor: 2`. Kept more + conservative than the connection-failure retry budget since retrying an + overloaded upstream aggressively can make things worse + +`Faraday::ResourceNotFound` (404) responses are not retried, since a 404 is +not a transient failure. --- diff --git a/lib/data_services_api/service.rb b/lib/data_services_api/service.rb index 908cef4..80b5cfa 100644 --- a/lib/data_services_api/service.rb +++ b/lib/data_services_api/service.rb @@ -1,15 +1,39 @@ # frozen_string_literal: true +require 'faraday' + module DataServicesApi # Denotes the encapsulated DataServicesAPI service class Service # rubocop:disable Metrics/ClassLength attr_reader :instrumenter, :logger, :parser, :url + DEFAULT_CONNECTION_FAILED_RETRY_OPTIONS = { + max: 4, + interval: 0.5, + interval_randomness: 0.25, + backoff_factor: 2, + exceptions: [Faraday::ConnectionFailed] + }.freeze + + DEFAULT_TIMEOUT_RETRY_OPTIONS = { + max: 2, + interval: 0.25, + interval_randomness: 0.5, + backoff_factor: 2, + exceptions: [Faraday::TimeoutError] + }.freeze + def initialize(config = {}) @instrumenter = config[:instrumenter] || (in_rails? && ActiveSupport::Notifications) @logger = config[:logger] || (in_rails? && Rails.logger) @parser = Yajl::Parser.new @url = config[:url] + @connection_failed_retry_options = DEFAULT_CONNECTION_FAILED_RETRY_OPTIONS.merge( + config[:connection_failed_retry_options] || {} + ) + @timeout_retry_options = DEFAULT_TIMEOUT_RETRY_OPTIONS.merge( + config[:timeout_retry_options] || {} + ) end def datasets @@ -153,14 +177,6 @@ def post_to_api(http_url, json) # rubocop:disable Metrics/AbcSize end def create_http_connection(http_url, auth = false) # rubocop:disable Metrics/MethodLength - retry_options = { - max: 2, - interval: 0.05, - interval_randomness: 0.5, - backoff_factor: 2, - exceptions: [Faraday::TimeoutError, Faraday::ConnectionFailed, Faraday::ResourceNotFound] - } - Faraday.new(url: http_url) do |config| config.use Faraday::Request::UrlEncoded config.use Faraday::FollowRedirects::Middleware @@ -168,7 +184,11 @@ def create_http_connection(http_url, auth = false) # rubocop:disable Metrics/Met config.request :authorization, :basic, api_user, api_pw if auth # instrument the request to log the time it takes to complete but only if we're in a Rails environment config.request :instrumentation, name: 'requests.api' if in_rails? - config.request :retry, retry_options + # Faraday::ResourceNotFound (404) is not transient and is deliberately not retried. + # Inner middleware exhausts its own budget before the exception reaches the next layer, + # so stack the more conservative timeout retry inside the more generous connection retry. + config.request :retry, @connection_failed_retry_options + config.request :retry, @timeout_retry_options config.response :json # ! Since responses are processed by the middleware stack in reverse order