From 03a712a53ebd6f9297e138451ba1b5a8475734af Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sat, 19 Oct 2024 21:11:33 +0900 Subject: [PATCH 1/4] Do not warn even if faraday-retry gem is missing It makes little sense to emit a warning saying "to use retry middleware" when the user does not request that. Remove the warning. --- lib/octokit/default.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/octokit/default.rb b/lib/octokit/default.rb index ca2e2fb73..59317b40d 100644 --- a/lib/octokit/default.rb +++ b/lib/octokit/default.rb @@ -6,14 +6,6 @@ require 'octokit/version' require 'octokit/warnable' -if Gem::Version.new(Faraday::VERSION) >= Gem::Version.new('2.0') - begin - require 'faraday/retry' - rescue LoadError - Octokit::Warnable.octokit_warn 'To use retry middleware with Faraday v2.0+, install `faraday-retry` gem' - end -end - module Octokit # Default configuration options for {Client} module Default From 2efe1b81822801516ec42f435a8d9d217c35aa2c Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sat, 19 Oct 2024 21:11:45 +0900 Subject: [PATCH 2/4] Duplicate the default middleware stack Duplicate the default middleware stack so that it won't be modified. --- lib/octokit/default.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/octokit/default.rb b/lib/octokit/default.rb index 59317b40d..0a7678dcc 100644 --- a/lib/octokit/default.rb +++ b/lib/octokit/default.rb @@ -139,7 +139,7 @@ def login # from {MIDDLEWARE} # @return [Faraday::RackBuilder or Faraday::Builder] def middleware - MIDDLEWARE + MIDDLEWARE.dup end # Default GitHub password for Basic Auth from ENV From c892eb3ad3e5e32e79c537f94df102947eaa92a2 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sat, 19 Oct 2024 21:11:55 +0900 Subject: [PATCH 3/4] Add Octokit::Middleware::Retry Octokit::Middleware::Retry allows incorporating the retry logic without relying on the default middleware stack. --- README.md | 2 +- lib/octokit.rb | 6 ++++++ lib/octokit/middleware/retry.rb | 19 +++++++++++++++++++ spec/octokit/client_spec.rb | 21 +++++++++++++++++++++ 4 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 lib/octokit/middleware/retry.rb diff --git a/README.md b/README.md index 569278ad7..afcb6c753 100644 --- a/README.md +++ b/README.md @@ -597,7 +597,7 @@ traffic: ```ruby stack = Faraday::RackBuilder.new do |builder| - builder.use Faraday::Retry::Middleware, exceptions: Faraday::Request::Retry::DEFAULT_EXCEPTIONS + [Octokit::ServerError] # or Faraday::Request::Retry for Faraday < 2.0 + builder.use Octokit::Middleware::Retry builder.use Octokit::Middleware::FollowRedirects builder.use Octokit::Response::RaiseError builder.use Octokit::Response::FeedParser diff --git a/lib/octokit.rb b/lib/octokit.rb index b029d8bc9..db9c9fcbd 100644 --- a/lib/octokit.rb +++ b/lib/octokit.rb @@ -76,6 +76,12 @@ def method_missing(method_name, *args, &block) super end end + + module Middleware + # In Faraday 2.x, Faraday::Request::Retry was moved to a separate gem + # so we use it only when requested. + autoload :Retry, 'octokit/middleware/retry' + end end Octokit.setup diff --git a/lib/octokit/middleware/retry.rb b/lib/octokit/middleware/retry.rb new file mode 100644 index 000000000..f442b16eb --- /dev/null +++ b/lib/octokit/middleware/retry.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Octokit + module Middleware + base = if defined?(Faraday::Request::Retry) + Faraday::Request::Retry + else + require 'faraday/retry' + Faraday::Retry::Middleware + end + + # Public: Retries each request a limited number of times. + class Retry < base + def initialize(app, **kwargs) + super(app, **kwargs, exceptions: DEFAULT_EXCEPTIONS + [Octokit::ServerError]) + end + end + end +end diff --git a/spec/octokit/client_spec.rb b/spec/octokit/client_spec.rb index 0ed906058..1c439315c 100644 --- a/spec/octokit/client_spec.rb +++ b/spec/octokit/client_spec.rb @@ -514,6 +514,27 @@ end end + describe 'retry' do + it 'retries for 504 response' do + client = oauth_client + client.middleware.insert Octokit::Response::RaiseError, Octokit::Middleware::Retry + + requested = false + + request = stub_get('/foo').to_return do + if requested + { status: 200 } + else + requested = true + { status: 504 } + end + end + + client.get('/foo') + assert_requested request, times: 2 + end + end + describe 'redirect handling' do it 'follows redirect for 301 response' do client = oauth_client From 3395ed41c0ee7dbc39856c96599acee181638b24 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sat, 19 Oct 2024 21:12:06 +0900 Subject: [PATCH 4/4] Avoid retrying by default No retry middleware is included in Faraday 2.x and retrying requires Faraday Retry gem. The default middlware stack used to include a retry middleware only when available, which means the behavior of Octokit implicitly changes depending on to the Faraday version or the presence of Faraday Retry gem. Remove the retry middleware from the default middleware stack to avoid such an implicit behavioral change. --- README.md | 17 +++++++++++++++++ lib/octokit/default.rb | 10 ---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index afcb6c753..ba731494f 100644 --- a/README.md +++ b/README.md @@ -631,6 +631,22 @@ x-ratelimit-reset: "1377205443" See the [Faraday README][faraday] for more middleware magic. +### Retrying + +If you want to retry requests, use `Octokit::Middleware::Retry`. + +It uses [Faraday Retry gem] for Faraday 2.x. Add the gem to your Gemfile + +```ruby +gem 'faraday-retry' +``` + +Next, insert `Octokit::Middleware::Retry` before `Octokit::Response::RaiseError`: + +```ruby +Octokit.middleware.insert Octokit::Response::RaiseError, Octokit::Middleware::Retry +``` + ### Caching If you want to boost performance, stretch your API rate limit, or avoid paying @@ -655,6 +671,7 @@ Once configured, the middleware will store responses in cache based on ETag fingerprint and serve those back up for future `304` responses for the same resource. See the [project README][cache] for advanced usage. +[retry]: https://github.com/lostisland/faraday-retry [cache]: https://github.com/sourcelevel/faraday-http-cache [faraday]: https://github.com/lostisland/faraday diff --git a/lib/octokit/default.rb b/lib/octokit/default.rb index 0a7678dcc..3d4d84833 100644 --- a/lib/octokit/default.rb +++ b/lib/octokit/default.rb @@ -23,16 +23,6 @@ module Default # Default Faraday middleware stack MIDDLEWARE = Faraday::RackBuilder.new do |builder| - # In Faraday 2.x, Faraday::Request::Retry was moved to a separate gem - # so we use it only when it's available. - if defined?(Faraday::Request::Retry) - retry_exceptions = Faraday::Request::Retry::DEFAULT_EXCEPTIONS + [Octokit::ServerError] - builder.use Faraday::Request::Retry, exceptions: retry_exceptions - elsif defined?(Faraday::Retry::Middleware) - retry_exceptions = Faraday::Retry::Middleware::DEFAULT_EXCEPTIONS + [Octokit::ServerError] - builder.use Faraday::Retry::Middleware, exceptions: retry_exceptions - end - builder.use Octokit::Middleware::FollowRedirects builder.use Octokit::Response::RaiseError builder.use Octokit::Response::FeedParser