From 16fbf00dc4b69cfb6eff116daa8f4843beaef8df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Mon, 16 Nov 2020 18:12:40 +0100 Subject: [PATCH] Merge pull request #4061 from rubygems/better_http_errors Always show underlying error when fetching specs fails (cherry picked from commit 1fa4ee3bff0c7b57f3484d3f05438cdc6b612146) --- bundler/lib/bundler/fetcher/index.rb | 5 +- bundler/spec/bundler/fetcher/index_spec.rb | 117 ++++++++------------ bundler/spec/realworld/mirror_probe_spec.rb | 28 ++--- 3 files changed, 65 insertions(+), 85 deletions(-) diff --git a/bundler/lib/bundler/fetcher/index.rb b/bundler/lib/bundler/fetcher/index.rb index 7e07eaea7b95..08b041897eb3 100644 --- a/bundler/lib/bundler/fetcher/index.rb +++ b/bundler/lib/bundler/fetcher/index.rb @@ -8,7 +8,7 @@ class Fetcher class Index < Base def specs(_gem_names) Bundler.rubygems.fetch_all_remote_specs(remote) - rescue Gem::RemoteFetcher::FetchError, OpenSSL::SSL::SSLError, Net::HTTPFatalError => e + rescue Gem::RemoteFetcher::FetchError => e case e.message when /certificate verify failed/ raise CertificateFailureError.new(display_uri) @@ -19,8 +19,7 @@ def specs(_gem_names) raise BadAuthenticationError, remote_uri if remote_uri.userinfo raise AuthenticationRequiredError, remote_uri else - Bundler.ui.trace e - raise HTTPError, "Could not fetch specs from #{display_uri}" + raise HTTPError, "Could not fetch specs from #{display_uri} due to underlying error <#{e.message}>" end end diff --git a/bundler/spec/bundler/fetcher/index_spec.rb b/bundler/spec/bundler/fetcher/index_spec.rb index 5ecd7d9e059e..b8ce46321e1b 100644 --- a/bundler/spec/bundler/fetcher/index_spec.rb +++ b/bundler/spec/bundler/fetcher/index_spec.rb @@ -17,100 +17,81 @@ end context "error handling" do - shared_examples_for "the error is properly handled" do - let(:remote_uri) { Bundler::URI("http://remote-uri.org") } - before do - allow(subject).to receive(:remote_uri).and_return(remote_uri) - end + let(:remote_uri) { Bundler::URI("http://remote-uri.org") } + before do + allow(rubygems).to receive(:fetch_all_remote_specs) { raise Gem::RemoteFetcher::FetchError.new(error_message, display_uri) } + allow(subject).to receive(:remote_uri).and_return(remote_uri) + end - context "when certificate verify failed" do - let(:error_message) { "certificate verify failed" } + context "when certificate verify failed" do + let(:error_message) { "certificate verify failed" } - it "should raise a Bundler::Fetcher::CertificateFailureError" do - expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::CertificateFailureError, - %r{Could not verify the SSL certificate for http://sample_uri.com}) - end + it "should raise a Bundler::Fetcher::CertificateFailureError" do + expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::CertificateFailureError, + %r{Could not verify the SSL certificate for http://sample_uri.com}) end + end - context "when a 401 response occurs" do - let(:error_message) { "401" } - - before do - allow(remote_uri).to receive(:userinfo).and_return(userinfo) - end - - context "and there was userinfo" do - let(:userinfo) { double(:userinfo) } + context "when a 401 response occurs" do + let(:error_message) { "401" } - it "should raise a Bundler::Fetcher::BadAuthenticationError" do - expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::BadAuthenticationError, - %r{Bad username or password for http://remote-uri.org}) - end - end + before do + allow(remote_uri).to receive(:userinfo).and_return(userinfo) + end - context "and there was no userinfo" do - let(:userinfo) { nil } + context "and there was userinfo" do + let(:userinfo) { double(:userinfo) } - it "should raise a Bundler::Fetcher::AuthenticationRequiredError" do - expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::AuthenticationRequiredError, - %r{Authentication is required for http://remote-uri.org}) - end + it "should raise a Bundler::Fetcher::BadAuthenticationError" do + expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::BadAuthenticationError, + %r{Bad username or password for http://remote-uri.org}) end end - context "when a 403 response occurs" do - let(:error_message) { "403" } + context "and there was no userinfo" do + let(:userinfo) { nil } - before do - allow(remote_uri).to receive(:userinfo).and_return(userinfo) + it "should raise a Bundler::Fetcher::AuthenticationRequiredError" do + expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::AuthenticationRequiredError, + %r{Authentication is required for http://remote-uri.org}) end + end + end - context "and there was userinfo" do - let(:userinfo) { double(:userinfo) } + context "when a 403 response occurs" do + let(:error_message) { "403" } - it "should raise a Bundler::Fetcher::BadAuthenticationError" do - expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::BadAuthenticationError, - %r{Bad username or password for http://remote-uri.org}) - end - end + before do + allow(remote_uri).to receive(:userinfo).and_return(userinfo) + end - context "and there was no userinfo" do - let(:userinfo) { nil } + context "and there was userinfo" do + let(:userinfo) { double(:userinfo) } - it "should raise a Bundler::Fetcher::AuthenticationRequiredError" do - expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::AuthenticationRequiredError, - %r{Authentication is required for http://remote-uri.org}) - end + it "should raise a Bundler::Fetcher::BadAuthenticationError" do + expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::BadAuthenticationError, + %r{Bad username or password for http://remote-uri.org}) end end - context "any other message is returned" do - let(:error_message) { "You get an error, you get an error!" } - - before { allow(Bundler).to receive(:ui).and_return(double(:trace => nil)) } + context "and there was no userinfo" do + let(:userinfo) { nil } - it "should raise a Bundler::HTTPError" do - expect { subject.specs(gem_names) }.to raise_error(Bundler::HTTPError, "Could not fetch specs from http://sample_uri.com") + it "should raise a Bundler::Fetcher::AuthenticationRequiredError" do + expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::AuthenticationRequiredError, + %r{Authentication is required for http://remote-uri.org}) end end end - context "when a Gem::RemoteFetcher::FetchError occurs" do - before { allow(rubygems).to receive(:fetch_all_remote_specs) { raise Gem::RemoteFetcher::FetchError.new(error_message, nil) } } + context "any other message is returned" do + let(:error_message) { "You get an error, you get an error!" } - it_behaves_like "the error is properly handled" - end + before { allow(Bundler).to receive(:ui).and_return(double(:trace => nil)) } - context "when a OpenSSL::SSL::SSLError occurs" do - before { allow(rubygems).to receive(:fetch_all_remote_specs) { raise OpenSSL::SSL::SSLError.new(error_message) } } - - it_behaves_like "the error is properly handled" - end - - context "when a Net::HTTPFatalError occurs" do - before { allow(rubygems).to receive(:fetch_all_remote_specs) { raise Net::HTTPFatalError.new(error_message, 404) } } - - it_behaves_like "the error is properly handled" + it "should raise a Bundler::HTTPError" do + expect { subject.specs(gem_names) }.to raise_error(Bundler::HTTPError, "Could not fetch specs from http://sample_uri.com due to underlying error ") + end end end end diff --git a/bundler/spec/realworld/mirror_probe_spec.rb b/bundler/spec/realworld/mirror_probe_spec.rb index 626092c7ebee..a2b5c89150db 100644 --- a/bundler/spec/realworld/mirror_probe_spec.rb +++ b/bundler/spec/realworld/mirror_probe_spec.rb @@ -74,10 +74,10 @@ bundle :install, :artifice => nil, :raise_on_error => false expect(out).to include("Fetching source index from #{mirror}") - expect(err).to include("Retrying fetcher due to error (2/4): Bundler::HTTPError Could not fetch specs from #{mirror}") - expect(err).to include("Retrying fetcher due to error (3/4): Bundler::HTTPError Could not fetch specs from #{mirror}") - expect(err).to include("Retrying fetcher due to error (4/4): Bundler::HTTPError Could not fetch specs from #{mirror}") - expect(err).to include("Could not fetch specs from #{mirror}") + expect(err).to include("Retrying fetcher due to error (2/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error ") + expect(err).to include("Retrying fetcher due to error (3/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error ") + expect(err).to include("Retrying fetcher due to error (4/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error ") + expect(err).to include("Could not fetch specs from #{mirror}/ due to underlying error ") end it "prints each error and warning on a new line" do @@ -90,10 +90,10 @@ expect(out).to include "Fetching source index from #{mirror}/" expect(err).to include <<-EOS.strip -Retrying fetcher due to error (2/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ -Retrying fetcher due to error (3/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ -Retrying fetcher due to error (4/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ -Could not fetch specs from #{mirror}/ +Retrying fetcher due to error (2/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error +Retrying fetcher due to error (3/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error +Retrying fetcher due to error (4/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error +Could not fetch specs from #{mirror}/ due to underlying error EOS end end @@ -112,10 +112,10 @@ bundle :install, :artifice => nil, :raise_on_error => false expect(out).to include("Fetching source index from #{mirror}") - expect(err).to include("Retrying fetcher due to error (2/4): Bundler::HTTPError Could not fetch specs from #{mirror}") - expect(err).to include("Retrying fetcher due to error (3/4): Bundler::HTTPError Could not fetch specs from #{mirror}") - expect(err).to include("Retrying fetcher due to error (4/4): Bundler::HTTPError Could not fetch specs from #{mirror}") - expect(err).to include("Could not fetch specs from #{mirror}") + expect(err).to include("Retrying fetcher due to error (2/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error ") + expect(err).to include("Retrying fetcher due to error (3/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error ") + expect(err).to include("Retrying fetcher due to error (4/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error ") + expect(err).to include("Could not fetch specs from #{mirror}/ due to underlying error ") end end @@ -138,7 +138,7 @@ def setup_server end def setup_mirror - mirror_port = find_unused_port - @mirror_uri = "http://#{host}:#{mirror_port}" + @mirror_port = find_unused_port + @mirror_uri = "http://#{host}:#{@mirror_port}" end end