Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Commit

Permalink
Retry fetch specs with --retry
Browse files Browse the repository at this point in the history
This PR adds a general purpose Retry class that can be re-used where-ever retry code is needed!

It also allows you to call `bundle install --retry 3` which will attempt to connect to ruby gems 3 times before failing, and emit a warning message each time
  • Loading branch information
schneems committed Sep 28, 2013
1 parent 1f6805e commit dc7dd9a
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 4 deletions.
1 change: 1 addition & 0 deletions lib/bundler.rb
Expand Up @@ -33,6 +33,7 @@ module Bundler
autoload :MatchPlatform, 'bundler/match_platform'
autoload :RemoteSpecification, 'bundler/remote_specification'
autoload :Resolver, 'bundler/resolver'
autoload :Retry, 'bundler/retry'
autoload :RubyVersion, 'bundler/ruby_version'
autoload :RubyDsl, 'bundler/ruby_dsl'
autoload :Runtime, 'bundler/runtime'
Expand Down
2 changes: 2 additions & 0 deletions lib/bundler/cli.rb
Expand Up @@ -180,6 +180,8 @@ def check
Bundler.rubygems.security_policies.empty?
method_option "jobs", :aliases => "-j", :type => :numeric, :banner =>
"Specify the number of jobs to run in parallel"
method_option "retry", :aliases => "-r", :type => :numeric, :banner =>
"Specify the number of times you wish to attempt a bundle install"

def install
opts = options.dup
Expand Down
16 changes: 12 additions & 4 deletions lib/bundler/installer.rb
Expand Up @@ -76,14 +76,22 @@ def run(options)
end
end

retry_times = options[:retry] || 1

# Since we are installing, we can resolve the definition
# using remote specs
unless local
options["local"] ?
@definition.resolve_with_cache! :
@definition.resolve_remotely!
if options["local"]
@definition.resolve_with_cache!
else
Bundler::Retry.new("source fetch", retry_times).attempts do
@definition.resolve_remotely!
end
end
end

# Must install gems in the order that the resolver provides
# as dependencies might actually affect the installation of
# the gem.
Installer.post_install_messages = {}

# the order that the resolver provides is significant, since
Expand Down
48 changes: 48 additions & 0 deletions lib/bundler/retry.rb
@@ -0,0 +1,48 @@
module Bundler
# General purpose class for retrying code that may fail
class Retry
attr_accessor :name, :max_attempts, :current_attempt

def initialize(name, max_attempts = 1)
@name = name
@max_attempts = max_attempts
end

def attempt(&block)
@current_attempt = 0
@failed = false
@error = nil
while keep_trying? do
run(&block)
end
@result
end
alias :attempts :attempt

private
def run(&block)
@failed = false
@current_attempt += 1
@result = block.call
rescue => e
fail(e)
end

def fail(e)
@failed = true
raise e if last_attempt?
return true unless name
Bundler.ui.warn "Retrying #{name} due to error (#{current_attempt.next}/#{max_attempts}): #{e.message}"
end

def keep_trying?
return true if current_attempt.zero?
return false if last_attempt?
return true if @failed
end

def last_attempt?
current_attempt >= max_attempts
end
end
end
36 changes: 36 additions & 0 deletions spec/bundler/retry_spec.rb
@@ -0,0 +1,36 @@
require 'spec_helper'

describe "bundle retry" do
it "return successful result if no errors" do
attempts = 0
result = Bundler::Retry.new(nil, 3).attempt do
attempts += 1
:success
end
expect(result).to eq(:success)
expect(attempts).to eq(1)
end

it "returns the first valid result" do
jobs = [->{ raise "foo" }, ->{ :bar }, ->{ raise "foo" }]
attempts = 0
result = Bundler::Retry.new(nil, 3).attempt do
attempts += 1
job = jobs.shift
job.call
end
expect(result).to eq(:bar)
expect(attempts).to eq(2)
end

it "raises the last error" do
attempts = 0
expect {
Bundler::Retry.new(nil, 3).attempt do
attempts += 1
raise Bundler::GemfileNotFound
end
}.to raise_error(Bundler::GemfileNotFound)
expect(attempts).to eq(3)
end
end
15 changes: 15 additions & 0 deletions spec/install/invalid_spec.rb
Expand Up @@ -33,3 +33,18 @@
expect(out).to match(/invalid symlink/)
end
end

describe "invalid or inaccessible gem source" do
it "can be retried" do
gemfile <<-G
source "file://#{gem_repo_missing}"
gem "rack"
gem "signed_gem"
G
bundle "install --retry 3"
exp = Regexp.escape("Retrying source fetch due to error (2/3)")
expect(out).to match(exp)
exp = Regexp.escape("Retrying source fetch due to error (3/3)")
expect(out).to match(exp)
end
end
4 changes: 4 additions & 0 deletions spec/support/path.rb
Expand Up @@ -48,6 +48,10 @@ def gem_repo1(*args)
tmp("gems/remote1", *args)
end

def gem_repo_missing(*args)
tmp("gems/missing", *args)
end

def gem_repo2(*args)
tmp("gems/remote2", *args)
end
Expand Down

3 comments on commit dc7dd9a

@indirect
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is actually the wrong place to retry. We should be retrying around the network requests themselves, rather than trying to resolve multiple times. This actually causes the resolver to do a complete run three times if there's an error in the resolve:

$ bundle install
Fetching gem metadata from https://rubygems.org/
Resolving dependencies...
Retrying source fetch due to error (2/3): Could not find gem 'sparks (~> 0.1) ruby' in the gems available on this machine.
Resolving dependencies...
Retrying source fetch due to error (3/3): Could not find gem 'sparks (~> 0.1) ruby' in the gems available on this machine.
Resolving dependencies...
Could not find gem 'sparks (~> 0.1) ruby' in the gems available on this machine.

Unfortunately that means I'm going to revert this before releasing 1.4 unless we can get something less intrusive before then.

/cc @schneems

@schneems
Copy link
Contributor Author

@schneems schneems commented on dc7dd9a Oct 6, 2013 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@indirect
Copy link
Member

@indirect indirect commented on dc7dd9a Oct 6, 2013 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.