Auto merge of #5928 - shayonj:s/download-retry, r=segiddins

Retry downloading gems consistently across all versions of RubyGems

### What was the end-user problem that led to this PR?

`Gem::RemoteFetcher::FetchError` failures were not being retried when downloading gems. Retrying downloading of gems for network blips is very ideal and helpful in CI environment.

### What was your diagnosis of the problem?

- The retry logic that existed (using `Bundler::Retry`) for `Bundler::RubygemsIntegration#download_gem` was not getting called, becase `Bundler.rubygems` defaulted to a sub class of `Bundler::RubygemsIntegration::MoreFuture`, which inherited from `Bundler::RubygemsIntegration::Future` and `#download_gem` in there didn't perform download with retry logic.
- Sending `Gem::RemoteFetcher::FetchError` as an argument to `Bundler::Retry` meant, `Bundler::Retry` would fail first thing,[ if exception being raised was the one being supplied](https://github.com/bundler/bundler/blob/5a61b65ad5ec1df1539ecf8bc5d124f2b254ba14/lib/bundler/retry.rb#L46..L49). The intent is to retry when downloading fails.

**Example of how sub classing issue looked like:**

```ruby
class RubygemsIntegration
  def download_gem
    "I retry when downloading"
  end
end

class Future < RubygemsIntegration
  def download_gem
    "I don't retry when downloading"
  end
end

class MoreFuture < Future
  # no download_gem
end

mf = MoreFuture.new #=> Bundler.rubygems where RubyGem version is > 2.1.0
mf.download_gem
 => "I don't retry when downloading"
```

**Example of Bundler::Retry**

```ruby
Bundler::Retry.new("download gem from", StandardError).attempts do
  puts "I will only print once, whereas I should print 4 times"
  raise StandardError.new("FooBar")
end
```
### What is your fix for the problem, implemented in this PR?

I added the retry logic for to `Bundler::RubygemsIntegration::Future#download_gem` and removed `Gem::RemoteFetcher::FetchError` as an argument for failed exceptions.

### Why did you choose this fix out of the possible options?

I chose this fix because this fix fulfills the expected behavior(https://github.com/bundler/bundler/issues/4846). Also added specs that makes sure retry logic is intact, which otherwise would have failed without the current logic in place.

1 files changed, 4 insertions, 2 deletions

diff --git a/lib/bundler/rubygems_integration.rb b/lib/bundler/rubygems_integration.rb
index 6654c292e6..dc1c35b990 100644
--- a/lib/bundler/rubygems_integration.rb
+++ b/lib/bundler/rubygems_integration.rb
@@ -317,7 +317,7 @@ module Bundler
     def download_gem(spec, uri, path)
       uri = Bundler.settings.mirror_for(uri)
       fetcher = Gem::RemoteFetcher.new(configuration[:http_proxy])
-      Bundler::Retry.new("download gem #{uri}", Gem::RemoteFetcher::FetchError).attempts do
+      Bundler::Retry.new("download gem from #{uri}").attempts do
         fetcher.download(spec, uri, path)
       end
     end
@@ -752,7 +752,9 @@ module Bundler
         uri = Bundler.settings.mirror_for(uri)
         fetcher = gem_remote_fetcher
         fetcher.headers = { "X-Gemfile-Source" => spec.remote.original_uri.to_s } if spec.remote.original_uri
-        fetcher.download(spec, uri, path)
+        Bundler::Retry.new("download gem from #{uri}").attempts do
+          fetcher.download(spec, uri, path)
+        end
       end
 
       def gem_remote_fetcher