From 3d7365292d5d4fd152c014bea01614df7fc695f0 Mon Sep 17 00:00:00 2001 From: Islam Elsayed Date: Wed, 9 Sep 2026 17:30:17 +0300 Subject: [PATCH] Apply BUNDLE_RETRY to lazy gemspec downloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolving through the full-index API lazily downloads individual gemspecs from /quick/Marshal.4.8/*.gemspec.rz. `Fetcher#fetch_spec` made that request bare: Bundler.safe_load_marshal Bundler.rubygems.inflate(downloader.fetch(uri).body) while `specs_with_retry`, in the same class, wraps its network call in `Bundler::Retry`. So a single transient failure — a read timeout, say — aborted the whole resolution, no matter what BUNDLE_RETRY was set to. That is most painful without a lockfile, where a resolve can fetch a great many gemspecs one at a time and any one of them can end it. Wrap the download in the same `Bundler::Retry.new("fetcher", FAIL_ERRORS)` the index path already uses. FAIL_ERRORS is what keeps this from retrying things that will not change, such as authentication failures. Only the download branch is wrapped. The file:// and cached-gemspec branches do no network I/O, so retrying them would just repeat a local failure. Closes #9817 --- lib/bundler/fetcher.rb | 6 +++++- spec/bundler/fetcher_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/bundler/fetcher.rb b/lib/bundler/fetcher.rb index 3d77f8750b8d..19b8a650f5fe 100644 --- a/lib/bundler/fetcher.rb +++ b/lib/bundler/fetcher.rb @@ -157,7 +157,11 @@ def fetch_spec(spec) elsif cached_spec_path = gemspec_cached_path(spec_file_name) Bundler.load_gemspec(cached_spec_path) else - Bundler.safe_load_marshal Bundler.rubygems.inflate(downloader.fetch(uri).body) + # Retried like `specs_with_retry`, which fetches the full index. + body = Bundler::Retry.new("fetcher", FAIL_ERRORS).attempts do + downloader.fetch(uri).body + end + Bundler.safe_load_marshal Bundler.rubygems.inflate(body) end raise MarshalError, "is #{spec.inspect}" unless spec.is_a?(Gem::Specification) spec diff --git a/spec/bundler/fetcher_spec.rb b/spec/bundler/fetcher_spec.rb index 8bac5548b924..88b6facefb83 100644 --- a/spec/bundler/fetcher_spec.rb +++ b/spec/bundler/fetcher_spec.rb @@ -205,6 +205,33 @@ def configured_connection end end + context "when the download fails transiently" do + let(:spec) { Gem::Specification.new(name, version) } + let(:downloaded_data) { Zlib::Deflate.deflate(Marshal.dump(spec)) } + + it "retries the download and returns the spec" do + expect(Bundler::Fetcher::Downloader).to receive(:new).and_return(downloader) + expect(downloader).to receive(:fetch).twice do + @attempts = (@attempts || 0) + 1 + raise Bundler::HTTPError, "transient network failure" if @attempts == 1 + body + end + + result = fetcher.fetch_spec([name, version, platform]) + expect(result).to eq(spec) + end + + it "does not retry an error that bypasses retries" do + expect(Bundler::Fetcher::Downloader).to receive(:new).and_return(downloader) + expect(downloader).to receive(:fetch).once.and_raise( + Bundler::Fetcher::AuthenticationRequiredError.new("http://example.org") + ) + + expect { fetcher.fetch_spec([name, version, platform]) }. + to raise_error(Bundler::Fetcher::AuthenticationRequiredError) + end + end + context "when attempting to load an unexpected class" do let(:downloaded_data) { Zlib::Deflate.deflate(Marshal.dump(3)) }