Skip to content

Commit

Permalink
Merge pull request #4061 from rubygems/better_http_errors
Browse files Browse the repository at this point in the history
Always show underlying error when fetching specs fails
  • Loading branch information
deivid-rodriguez committed Nov 16, 2020
2 parents fcdcfc0 + 954c543 commit 1fa4ee3
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 85 deletions.
5 changes: 2 additions & 3 deletions bundler/lib/bundler/fetcher/index.rb
Expand Up @@ -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)
Expand All @@ -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

Expand Down
117 changes: 49 additions & 68 deletions bundler/spec/bundler/fetcher/index_spec.rb
Expand Up @@ -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 <You get an error, you get an error! (http://sample_uri.com)>")
end
end
end
end
28 changes: 14 additions & 14 deletions bundler/spec/realworld/mirror_probe_spec.rb
Expand Up @@ -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 <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>")
expect(err).to include("Retrying fetcher due to error (3/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>")
expect(err).to include("Retrying fetcher due to error (4/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>")
expect(err).to include("Could not fetch specs from #{mirror}/ due to underlying error <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>")
end

it "prints each error and warning on a new line" do
Expand All @@ -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 <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>
Retrying fetcher due to error (3/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>
Retrying fetcher due to error (4/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>
Could not fetch specs from #{mirror}/ due to underlying error <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>
EOS
end
end
Expand All @@ -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 <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>")
expect(err).to include("Retrying fetcher due to error (3/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>")
expect(err).to include("Retrying fetcher due to error (4/4): Bundler::HTTPError Could not fetch specs from #{mirror}/ due to underlying error <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>")
expect(err).to include("Could not fetch specs from #{mirror}/ due to underlying error <Errno::ECONNREFUSED: Failed to open TCP connection to #{host}:#{@mirror_port} (Connection refused - connect(2) for \"#{host}\" port #{@mirror_port}) (#{mirror}/specs.4.8.gz)>")
end
end

Expand All @@ -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

0 comments on commit 1fa4ee3

Please sign in to comment.