diff --git a/bundler/lib/bundler/retry.rb b/bundler/lib/bundler/retry.rb index 090cb7e2cae1..bbd1b9899649 100644 --- a/bundler/lib/bundler/retry.rb +++ b/bundler/lib/bundler/retry.rb @@ -6,6 +6,8 @@ class Retry attr_accessor :name, :total_runs, :current_run class << self + attr_accessor :default_base_delay + def default_attempts default_retries + 1 end @@ -16,11 +18,17 @@ def default_retries end end - def initialize(name, exceptions = nil, retries = self.class.default_retries) + # Set default base delay for exponential backoff + self.default_base_delay = 1.0 + + def initialize(name, exceptions = nil, retries = self.class.default_retries, opts = {}) @name = name @retries = retries @exceptions = Array(exceptions) || [] @total_runs = @retries + 1 # will run once, then upto attempts.times + @base_delay = opts[:base_delay] || self.class.default_base_delay + @max_delay = opts[:max_delay] || 60.0 + @jitter = opts[:jitter] || 0.5 end def attempt(&block) @@ -48,9 +56,23 @@ def fail_attempt(e) Bundler.ui.info "" unless Bundler.ui.debug? raise e end - return true unless name - Bundler.ui.info "" unless Bundler.ui.debug? # Add new line in case dots preceded this - Bundler.ui.warn "Retrying #{name} due to error (#{current_run.next}/#{total_runs}): #{e.class} #{e.message}", true + if name + Bundler.ui.info "" unless Bundler.ui.debug? # Add new line in case dots preceded this + Bundler.ui.warn "Retrying #{name} due to error (#{current_run.next}/#{total_runs}): #{e.class} #{e.message}", true + end + backoff_sleep if @base_delay > 0 + true + end + + def backoff_sleep + # Exponential backoff: delay = base_delay * 2^(attempt - 1) + # Add jitter to prevent thundering herd: random value between 0 and jitter seconds + delay = @base_delay * (2**(@current_run - 1)) + delay = [@max_delay, delay].min + jitter_amount = rand * @jitter + total_delay = delay + jitter_amount + Bundler.ui.debug "Sleeping for #{total_delay.round(2)} seconds before retry" + sleep(total_delay) end def keep_trying? diff --git a/bundler/spec/bundler/retry_spec.rb b/bundler/spec/bundler/retry_spec.rb index 7481622a967d..5dbf58599af2 100644 --- a/bundler/spec/bundler/retry_spec.rb +++ b/bundler/spec/bundler/retry_spec.rb @@ -78,4 +78,108 @@ end end end + + context "exponential backoff" do + it "can be disabled by setting base_delay to 0" do + attempts = 0 + expect do + Bundler::Retry.new("test", [], 2, base_delay: 0).attempt do + attempts += 1 + raise "error" + end + end.to raise_error(StandardError) + + # Verify no sleep was called (implicitly - if sleep was called, timing would be different) + expect(attempts).to eq(3) + end + + it "is enabled by default with 1 second base delay" do + attempts = 0 + sleep_times = [] + + allow_any_instance_of(Bundler::Retry).to receive(:sleep) do |_instance, delay| + sleep_times << delay + end + + expect do + Bundler::Retry.new("test", [], 2, jitter: 0).attempt do + attempts += 1 + raise "error" + end + end.to raise_error(StandardError) + + expect(attempts).to eq(3) + expect(sleep_times.length).to eq(2) + # First retry: 1.0 * 2^0 = 1.0 + expect(sleep_times[0]).to eq(1.0) + # Second retry: 1.0 * 2^1 = 2.0 + expect(sleep_times[1]).to eq(2.0) + end + + it "sleeps with exponential backoff when base_delay is set" do + attempts = 0 + sleep_times = [] + + allow_any_instance_of(Bundler::Retry).to receive(:sleep) do |_instance, delay| + sleep_times << delay + end + + expect do + Bundler::Retry.new("test", [], 2, base_delay: 1.0, jitter: 0).attempt do + attempts += 1 + raise "error" + end + end.to raise_error(StandardError) + + expect(attempts).to eq(3) + expect(sleep_times.length).to eq(2) + # First retry: 1.0 * 2^0 = 1.0 + expect(sleep_times[0]).to eq(1.0) + # Second retry: 1.0 * 2^1 = 2.0 + expect(sleep_times[1]).to eq(2.0) + end + + it "respects max_delay" do + sleep_times = [] + + allow_any_instance_of(Bundler::Retry).to receive(:sleep) do |_instance, delay| + sleep_times << delay + end + + expect do + Bundler::Retry.new("test", [], 3, base_delay: 10.0, max_delay: 15.0, jitter: 0).attempt do + raise "error" + end + end.to raise_error(StandardError) + + # First retry: 10.0 * 2^0 = 10.0 + expect(sleep_times[0]).to eq(10.0) + # Second retry: 10.0 * 2^1 = 20.0, capped at 15.0 + expect(sleep_times[1]).to eq(15.0) + # Third retry: 10.0 * 2^2 = 40.0, capped at 15.0 + expect(sleep_times[2]).to eq(15.0) + end + + it "adds jitter to delay" do + sleep_times = [] + + allow_any_instance_of(Bundler::Retry).to receive(:sleep) do |_instance, delay| + sleep_times << delay + end + + expect do + Bundler::Retry.new("test", [], 2, base_delay: 1.0, jitter: 0.5).attempt do + raise "error" + end + end.to raise_error(StandardError) + + expect(sleep_times.length).to eq(2) + # First retry should be between 1.0 and 1.5 (base + jitter) + expect(sleep_times[0]).to be >= 1.0 + expect(sleep_times[0]).to be <= 1.5 + # Second retry should be between 2.0 and 2.5 + expect(sleep_times[1]).to be >= 2.0 + expect(sleep_times[1]).to be <= 2.5 + end + end end diff --git a/bundler/spec/spec_helper.rb b/bundler/spec/spec_helper.rb index a79e33fbb016..bbbc75d2ccf7 100644 --- a/bundler/spec/spec_helper.rb +++ b/bundler/spec/spec_helper.rb @@ -93,6 +93,9 @@ def self.ruby=(ruby) require_relative "support/rubygems_ext" Spec::Rubygems.test_setup + # Disable retry delays in tests to speed them up + Bundler::Retry.default_base_delay = 0 + # Simulate bundler has not yet been loaded ENV.replace(ENV.to_hash.delete_if {|k, _v| k.start_with?(Bundler::EnvironmentPreserver::BUNDLER_PREFIX) })