From 9d2cbb1b176126df2d1528fe6332250546047852 Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Sat, 6 Apr 2019 17:15:55 +0530 Subject: Rescue connection exceptions - Rescue a bunch of exceptions when making a request to Sentry. - Also add a spec for checking for handling of MissingKeysError in the list_sentry_projects_service.rb --- .../error_tracking/list_projects_service.rb | 2 + lib/sentry/client.rb | 13 +++++- spec/lib/sentry/client_spec.rb | 52 ++++++++++++---------- .../error_tracking/list_projects_service_spec.rb | 26 ++++++++--- 4 files changed, 62 insertions(+), 31 deletions(-) diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb index 82f456825a0..8d08f0cda94 100644 --- a/app/services/error_tracking/list_projects_service.rb +++ b/app/services/error_tracking/list_projects_service.rb @@ -15,6 +15,8 @@ module ErrorTracking result = setting.list_sentry_projects rescue Sentry::Client::Error => e return error(e.message, :bad_request) + rescue Sentry::Client::MissingKeysError => e + return error(e.message, :internal_server_error) end success(projects: result[:projects]) diff --git a/lib/sentry/client.rb b/lib/sentry/client.rb index 8665ea591a2..7cee14bd1f8 100644 --- a/lib/sentry/client.rb +++ b/lib/sentry/client.rb @@ -67,8 +67,17 @@ module Sentry def handle_request_exceptions yield - rescue => e - raise_error "Sentry request failed due to #{e.class}" + rescue HTTParty::Error => e + Gitlab::Sentry.track_acceptable_exception(e) + raise_error 'Error when connecting to Sentry' + rescue Net::OpenTimeout + raise_error 'Connection to Sentry timed out' + rescue SocketError + raise_error 'Received SocketError when trying to connect to Sentry' + rescue OpenSSL::SSL::SSLError + raise_error 'Sentry returned invalid SSL data' + rescue Errno::ECONNREFUSED + raise_error 'Connection refused' end def handle_response(response) diff --git a/spec/lib/sentry/client_spec.rb b/spec/lib/sentry/client_spec.rb index 972c23e485c..61296271fa7 100644 --- a/spec/lib/sentry/client_spec.rb +++ b/spec/lib/sentry/client_spec.rb @@ -61,20 +61,36 @@ describe Sentry::Client do end end - shared_examples 'maps exceptions' do |message| - it 'calls sentry api' do - expect { subject } - .to raise_exception(Sentry::Client::Error, message) + shared_examples 'maps exceptions' do + exceptions = { + HTTParty::Error => 'Error when connecting to Sentry', + Net::OpenTimeout => 'Connection to Sentry timed out', + SocketError => 'Received SocketError when trying to connect to Sentry', + OpenSSL::SSL::SSLError => 'Sentry returned invalid SSL data', + Errno::ECONNREFUSED => 'Connection refused' + } + + exceptions.each do |exception, message| + context "#{exception}" do + before do + stub_request(:get, sentry_request_url).to_raise(exception) + end + + it do + expect { subject } + .to raise_exception(Sentry::Client::Error, message) + end + end end end describe '#list_issues' do let(:issue_status) { 'unresolved' } let(:limit) { 20 } - let(:sentry_api_response) { issues_sample_response } + let(:sentry_request_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' } - let!(:sentry_api_request) { stub_sentry_request(sentry_url + '/issues/?limit=20&query=is:unresolved', body: sentry_api_response) } + let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response) } subject { client.list_issues(issue_status: issue_status, limit: limit) } @@ -128,16 +144,14 @@ describe Sentry::Client do # Sentry API returns 404 if there are extra slashes in the URL! context 'extra slashes in URL' do let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects//sentry-org/sentry-project/' } - let(:client) { described_class.new(sentry_url, token) } - let!(:valid_req_stub) do - stub_sentry_request( - 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/' \ + let(:sentry_request_url) do + 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/' \ 'issues/?limit=20&query=is:unresolved' - ) end it 'removes extra slashes in api url' do + expect(client.url).to eq(sentry_url) expect(Gitlab::HTTP).to receive(:get).with( URI('https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/issues/'), anything @@ -145,7 +159,7 @@ describe Sentry::Client do subject - expect(valid_req_stub).to have_been_requested + expect(sentry_api_request).to have_been_requested end end @@ -177,13 +191,7 @@ describe Sentry::Client do end end - context 'when exception is raised' do - before do - stub_sentry_request_raising(StandardError.new('ignore message')) - end - - it_behaves_like 'maps exceptions', 'Sentry request failed due to StandardError' - end + it_behaves_like 'maps exceptions' end describe '#list_projects' do @@ -277,11 +285,9 @@ describe Sentry::Client do end context 'when exception is raised' do - before do - stub_sentry_request_raising(StandardError.new('ignore message')) - end + let(:sentry_request_url) { sentry_list_projects_url } - it_behaves_like 'maps exceptions', 'Sentry request failed due to StandardError' + it_behaves_like 'maps exceptions' end end diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb index a92d3376f7b..730fccc599e 100644 --- a/spec/services/error_tracking/list_projects_service_spec.rb +++ b/spec/services/error_tracking/list_projects_service_spec.rb @@ -51,14 +51,28 @@ describe ErrorTracking::ListProjectsService do end context 'sentry client raises exception' do - before do - expect(error_tracking_setting).to receive(:list_sentry_projects) - .and_raise(Sentry::Client::Error, 'Sentry response status code: 500') + context 'Sentry::Client::Error' do + before do + expect(error_tracking_setting).to receive(:list_sentry_projects) + .and_raise(Sentry::Client::Error, 'Sentry response status code: 500') + end + + it 'returns error response' do + expect(result[:message]).to eq('Sentry response status code: 500') + expect(result[:http_status]).to eq(:bad_request) + end end - it 'returns error response' do - expect(result[:message]).to eq('Sentry response status code: 500') - expect(result[:http_status]).to eq(:bad_request) + context 'Sentry::Client::MissingKeysError' do + before do + expect(error_tracking_setting).to receive(:list_sentry_projects) + .and_raise(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "id"') + end + + it 'returns error response' do + expect(result[:message]).to eq('Sentry API response is missing keys. key not found: "id"') + expect(result[:http_status]).to eq(:internal_server_error) + end end end -- cgit v1.2.1