diff options
| -rw-r--r-- | app/controllers/import/bitbucket_server_controller.rb | 4 | ||||
| -rw-r--r-- | changelogs/unreleased/sh-fix-bitbucket-server-error-handling.yml | 5 | ||||
| -rw-r--r-- | lib/bitbucket_server/client.rb | 14 | ||||
| -rw-r--r-- | lib/bitbucket_server/connection.rb | 17 | ||||
| -rw-r--r-- | spec/controllers/import/bitbucket_server_controller_spec.rb | 2 | ||||
| -rw-r--r-- | spec/lib/bitbucket_server/client_spec.rb | 6 | ||||
| -rw-r--r-- | spec/lib/bitbucket_server/connection_spec.rb | 18 |
7 files changed, 43 insertions, 23 deletions
diff --git a/app/controllers/import/bitbucket_server_controller.rb b/app/controllers/import/bitbucket_server_controller.rb index 575c40d5f6f..87338488eba 100644 --- a/app/controllers/import/bitbucket_server_controller.rb +++ b/app/controllers/import/bitbucket_server_controller.rb @@ -40,7 +40,7 @@ class Import::BitbucketServerController < Import::BaseController else render json: { errors: 'This namespace has already been taken! Please choose another one.' }, status: :unprocessable_entity end - rescue BitbucketServer::Client::ServerError => e + rescue BitbucketServer::Connection::ConnectionError => e render json: { errors: "Unable to connect to server: #{e}" }, status: :unprocessable_entity end @@ -62,7 +62,7 @@ class Import::BitbucketServerController < Import::BaseController already_added_projects_names = @already_added_projects.pluck(:import_source) @repos.reject! { |repo| already_added_projects_names.include?(repo.browse_url) } - rescue BitbucketServer::Connection::ConnectionError, BitbucketServer::Client::ServerError => e + rescue BitbucketServer::Connection::ConnectionError => e flash[:alert] = "Unable to connect to server: #{e}" clear_session_data redirect_to new_import_bitbucket_server_path diff --git a/changelogs/unreleased/sh-fix-bitbucket-server-error-handling.yml b/changelogs/unreleased/sh-fix-bitbucket-server-error-handling.yml new file mode 100644 index 00000000000..87405fa0a78 --- /dev/null +++ b/changelogs/unreleased/sh-fix-bitbucket-server-error-handling.yml @@ -0,0 +1,5 @@ +--- +title: Fix Bitbucket Server importer error handling +merge_request: 24343 +author: +type: fixed diff --git a/lib/bitbucket_server/client.rb b/lib/bitbucket_server/client.rb index 83e8808db07..6a608058813 100644 --- a/lib/bitbucket_server/client.rb +++ b/lib/bitbucket_server/client.rb @@ -4,18 +4,6 @@ module BitbucketServer class Client attr_reader :connection - ServerError = Class.new(StandardError) - - SERVER_ERRORS = [SocketError, - OpenSSL::SSL::SSLError, - Errno::ECONNRESET, - Errno::ECONNREFUSED, - Errno::EHOSTUNREACH, - Net::OpenTimeout, - Net::ReadTimeout, - Gitlab::HTTP::BlockedUrlError, - BitbucketServer::Connection::ConnectionError].freeze - def initialize(options = {}) @connection = Connection.new(options) end @@ -64,8 +52,6 @@ module BitbucketServer def get_collection(path, type, page_offset: 0, limit: nil) paginator = BitbucketServer::Paginator.new(connection, Addressable::URI.escape(path), type, page_offset: page_offset, limit: limit) BitbucketServer::Collection.new(paginator) - rescue *SERVER_ERRORS => e - raise ServerError, e end end end diff --git a/lib/bitbucket_server/connection.rb b/lib/bitbucket_server/connection.rb index 7efcdcf8619..9c14b26c65a 100644 --- a/lib/bitbucket_server/connection.rb +++ b/lib/bitbucket_server/connection.rb @@ -7,6 +7,17 @@ module BitbucketServer DEFAULT_API_VERSION = '1.0' SEPARATOR = '/' + NETWORK_ERRORS = [ + SocketError, + OpenSSL::SSL::SSLError, + Errno::ECONNRESET, + Errno::ECONNREFUSED, + Errno::EHOSTUNREACH, + Net::OpenTimeout, + Net::ReadTimeout, + Gitlab::HTTP::BlockedUrlError + ].freeze + attr_reader :api_version, :base_uri, :username, :token ConnectionError = Class.new(StandardError) @@ -27,6 +38,8 @@ module BitbucketServer check_errors!(response) response.parsed_response + rescue *NETWORK_ERRORS => e + raise ConnectionError, e end def post(path, body) @@ -38,6 +51,8 @@ module BitbucketServer check_errors!(response) response.parsed_response + rescue *NETWORK_ERRORS => e + raise ConnectionError, e end # We need to support two different APIs for deletion: @@ -55,6 +70,8 @@ module BitbucketServer check_errors!(response) response.parsed_response + rescue *NETWORK_ERRORS => e + raise ConnectionError, e end private diff --git a/spec/controllers/import/bitbucket_server_controller_spec.rb b/spec/controllers/import/bitbucket_server_controller_spec.rb index 73195463a50..bb282db5a41 100644 --- a/spec/controllers/import/bitbucket_server_controller_spec.rb +++ b/spec/controllers/import/bitbucket_server_controller_spec.rb @@ -78,7 +78,7 @@ describe Import::BitbucketServerController do end it "returns an error when the server can't be contacted" do - expect(client).to receive(:repo).with(project_key, repo_slug).and_raise(BitbucketServer::Client::ServerError) + expect(client).to receive(:repo).with(project_key, repo_slug).and_raise(::BitbucketServer::Connection::ConnectionError) post :create, params: { project: project_key, repository: repo_slug }, format: :json diff --git a/spec/lib/bitbucket_server/client_spec.rb b/spec/lib/bitbucket_server/client_spec.rb index 5de0a9a65b5..b021e69800a 100644 --- a/spec/lib/bitbucket_server/client_spec.rb +++ b/spec/lib/bitbucket_server/client_spec.rb @@ -17,12 +17,6 @@ describe BitbucketServer::Client do subject.pull_requests(project, repo_slug) end - - it 'throws an exception when connection fails' do - allow(BitbucketServer::Collection).to receive(:new).and_raise(OpenSSL::SSL::SSLError) - - expect { subject.pull_requests(project, repo_slug) }.to raise_error(described_class::ServerError) - end end describe '#activities' do diff --git a/spec/lib/bitbucket_server/connection_spec.rb b/spec/lib/bitbucket_server/connection_spec.rb index b5da4cb1a49..ab8a5b89608 100644 --- a/spec/lib/bitbucket_server/connection_spec.rb +++ b/spec/lib/bitbucket_server/connection_spec.rb @@ -26,6 +26,12 @@ describe BitbucketServer::Connection do expect { subject.get(url) }.to raise_error(described_class::ConnectionError) end + + it 'throws an exception upon a network error' do + WebMock.stub_request(:get, url).with(headers: { 'Accept' => 'application/json' }).to_raise(OpenSSL::SSL::SSLError) + + expect { subject.get(url) }.to raise_error(described_class::ConnectionError) + end end describe '#post' do @@ -42,6 +48,12 @@ describe BitbucketServer::Connection do expect { subject.post(url, payload) }.to raise_error(described_class::ConnectionError) end + + it 'throws an exception upon a network error' do + WebMock.stub_request(:post, url).with(headers: { 'Accept' => 'application/json' }).to_raise(OpenSSL::SSL::SSLError) + + expect { subject.post(url, payload) }.to raise_error(described_class::ConnectionError) + end end describe '#delete' do @@ -63,6 +75,12 @@ describe BitbucketServer::Connection do expect { subject.delete(:branches, branch_path, payload) }.to raise_error(described_class::ConnectionError) end + + it 'throws an exception upon a network error' do + WebMock.stub_request(:delete, branch_url).with(headers: headers).to_raise(OpenSSL::SSL::SSLError) + + expect { subject.delete(:branches, branch_path, payload) }.to raise_error(described_class::ConnectionError) + end end end end |