summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Kozono <mkozono@gmail.com>2019-04-07 07:51:37 +0000
committerMichael Kozono <mkozono@gmail.com>2019-04-07 07:51:37 +0000
commitc62ef08a40b6213b8df3fe47a1c6104d0d693a8e (patch)
tree617df6d7bd4186c7f1eec0374e975375323d20b9
parentae91b3219aa0b5de20e3452126384341acef75c6 (diff)
parentbbb17ea1ea619664b362a9c984da45b940c412a2 (diff)
downloadgitlab-ce-c62ef08a40b6213b8df3fe47a1c6104d0d693a8e.tar.gz
Merge branch '60149-nameerror-uninitialized-constant-sentry-client-sentryerror' into 'master'
Handle possible HTTP exception for Sentry client Closes #60149 See merge request gitlab-org/gitlab-ce!27080
-rw-r--r--app/services/error_tracking/list_projects_service.rb4
-rw-r--r--changelogs/unreleased/60149-nameerror-uninitialized-constant-sentry-client-sentryerror.yml5
-rw-r--r--lib/sentry/client.rb30
-rw-r--r--spec/lib/sentry/client_spec.rb48
-rw-r--r--spec/services/error_tracking/list_projects_service_spec.rb26
5 files changed, 93 insertions, 20 deletions
diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb
index 4e92353a13c..8d08f0cda94 100644
--- a/app/services/error_tracking/list_projects_service.rb
+++ b/app/services/error_tracking/list_projects_service.rb
@@ -15,8 +15,8 @@ module ErrorTracking
result = setting.list_sentry_projects
rescue Sentry::Client::Error => e
return error(e.message, :bad_request)
- rescue Sentry::Client::SentryError => e
- return error(e.message, :unprocessable_entity)
+ rescue Sentry::Client::MissingKeysError => e
+ return error(e.message, :internal_server_error)
end
success(projects: result[:projects])
diff --git a/changelogs/unreleased/60149-nameerror-uninitialized-constant-sentry-client-sentryerror.yml b/changelogs/unreleased/60149-nameerror-uninitialized-constant-sentry-client-sentryerror.yml
new file mode 100644
index 00000000000..8c3a47cf62c
--- /dev/null
+++ b/changelogs/unreleased/60149-nameerror-uninitialized-constant-sentry-client-sentryerror.yml
@@ -0,0 +1,5 @@
+---
+title: Handle possible HTTP exception for Sentry client
+merge_request: 27080
+author:
+type: fixed
diff --git a/lib/sentry/client.rb b/lib/sentry/client.rb
index bb1aa2a7a10..4022e8ff946 100644
--- a/lib/sentry/client.rb
+++ b/lib/sentry/client.rb
@@ -47,9 +47,11 @@ module Sentry
end
def http_get(url, params = {})
- resp = Gitlab::HTTP.get(url, **request_params.merge(params))
+ response = handle_request_exceptions do
+ Gitlab::HTTP.get(url, **request_params.merge(params))
+ end
- handle_response(resp)
+ handle_response(response)
end
def get_issues(issue_status:, limit:)
@@ -63,14 +65,36 @@ module Sentry
http_get(projects_api_url)
end
+ def handle_request_exceptions
+ yield
+ 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'
+ rescue => e
+ Gitlab::Sentry.track_acceptable_exception(e)
+ raise_error "Sentry request failed due to #{e.class}"
+ end
+
def handle_response(response)
unless response.code == 200
- raise Client::Error, "Sentry response status code: #{response.code}"
+ raise_error "Sentry response status code: #{response.code}"
end
response
end
+ def raise_error(message)
+ raise Client::Error, message
+ end
+
def projects_api_url
projects_url = URI(@url)
projects_url.path = '/api/0/projects/'
diff --git a/spec/lib/sentry/client_spec.rb b/spec/lib/sentry/client_spec.rb
index 3333f8307ae..cb14204b99a 100644
--- a/spec/lib/sentry/client_spec.rb
+++ b/spec/lib/sentry/client_spec.rb
@@ -61,13 +61,37 @@ describe Sentry::Client do
end
end
+ 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',
+ StandardError => 'Sentry request failed due to StandardError'
+ }
+
+ 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) }
@@ -121,16 +145,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
@@ -138,7 +160,7 @@ describe Sentry::Client do
subject
- expect(valid_req_stub).to have_been_requested
+ expect(sentry_api_request).to have_been_requested
end
end
@@ -169,6 +191,8 @@ describe Sentry::Client do
expect { subject }.to raise_error(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "id"')
end
end
+
+ it_behaves_like 'maps exceptions'
end
describe '#list_projects' do
@@ -260,12 +284,18 @@ describe Sentry::Client do
expect(valid_req_stub).to have_been_requested
end
end
+
+ context 'when exception is raised' do
+ let(:sentry_request_url) { sentry_list_projects_url }
+
+ it_behaves_like 'maps exceptions'
+ end
end
private
def stub_sentry_request(url, body: {}, status: 200, headers: {})
- WebMock.stub_request(:get, url)
+ stub_request(:get, url)
.to_return(
status: status,
headers: { 'Content-Type' => 'application/json' }.merge(headers),
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