Commit 53646329 authored by Reuben Pereira's avatar Reuben Pereira Committed by Kamil Trzciński

Catch exception in calculate reactive cache

Return the exception as an error.
parent 56b82db6
...@@ -2,7 +2,7 @@ import Service from '../services'; ...@@ -2,7 +2,7 @@ import Service from '../services';
import * as types from './mutation_types'; import * as types from './mutation_types';
import createFlash from '~/flash'; import createFlash from '~/flash';
import Poll from '~/lib/utils/poll'; import Poll from '~/lib/utils/poll';
import { __ } from '~/locale'; import { __, sprintf } from '~/locale';
let eTagPoll; let eTagPoll;
...@@ -19,9 +19,17 @@ export function startPolling({ commit }, endpoint) { ...@@ -19,9 +19,17 @@ export function startPolling({ commit }, endpoint) {
commit(types.SET_EXTERNAL_URL, data.external_url); commit(types.SET_EXTERNAL_URL, data.external_url);
commit(types.SET_LOADING, false); commit(types.SET_LOADING, false);
}, },
errorCallback: () => { errorCallback: response => {
let errorMessage = '';
if (response && response.data && response.data.message) {
errorMessage = response.data.message;
}
commit(types.SET_LOADING, false); commit(types.SET_LOADING, false);
createFlash(__('Failed to load errors from Sentry')); createFlash(
sprintf(__(`Failed to load errors from Sentry. Error message: %{errorMessage}`), {
errorMessage,
}),
);
}, },
}); });
......
...@@ -58,7 +58,7 @@ module ErrorTracking ...@@ -58,7 +58,7 @@ module ErrorTracking
def list_sentry_issues(opts = {}) def list_sentry_issues(opts = {})
with_reactive_cache('list_issues', opts.stringify_keys) do |result| with_reactive_cache('list_issues', opts.stringify_keys) do |result|
{ issues: result } result
end end
end end
...@@ -69,8 +69,10 @@ module ErrorTracking ...@@ -69,8 +69,10 @@ module ErrorTracking
def calculate_reactive_cache(request, opts) def calculate_reactive_cache(request, opts)
case request case request
when 'list_issues' when 'list_issues'
sentry_client.list_issues(**opts.symbolize_keys) { issues: sentry_client.list_issues(**opts.symbolize_keys) }
end end
rescue Sentry::Client::Error => e
{ error: e.message }
end end
# http://HOST/api/0/projects/ORG/PROJECT # http://HOST/api/0/projects/ORG/PROJECT
......
...@@ -6,15 +6,19 @@ module ErrorTracking ...@@ -6,15 +6,19 @@ module ErrorTracking
DEFAULT_LIMIT = 20 DEFAULT_LIMIT = 20
def execute def execute
return error('not enabled') unless enabled? return error('Error Tracking is not enabled') unless enabled?
return error('access denied') unless can_read? return error('Access denied', :unauthorized) unless can_read?
result = project_error_tracking_setting result = project_error_tracking_setting
.list_sentry_issues(issue_status: issue_status, limit: limit) .list_sentry_issues(issue_status: issue_status, limit: limit)
# our results are not yet ready # our results are not yet ready
unless result unless result
return error('not ready', :no_content) return error('Not ready. Try again later', :no_content)
end
if result[:error].present?
return error(result[:error], :bad_request)
end end
success(issues: result[:issues]) success(issues: result[:issues])
......
---
title: Display error message when API call to list Sentry issues fails
merge_request: 24936
author:
type: fixed
...@@ -54,7 +54,7 @@ module Sentry ...@@ -54,7 +54,7 @@ module Sentry
def handle_response(response) def handle_response(response)
unless response.code == 200 unless response.code == 200
raise Client::Error, "Sentry response error: #{response.code}" raise Client::Error, "Sentry response status code: #{response.code}"
end end
response.as_json response.as_json
......
...@@ -3295,7 +3295,7 @@ msgstr "" ...@@ -3295,7 +3295,7 @@ msgstr ""
msgid "Failed to load emoji list." msgid "Failed to load emoji list."
msgstr "" msgstr ""
msgid "Failed to load errors from Sentry" msgid "Failed to load errors from Sentry. Error message: %{errorMessage}"
msgstr "" msgstr ""
msgid "Failed to remove issue from board, please try again." msgid "Failed to remove issue from board, please try again."
......
...@@ -36,7 +36,7 @@ describe Sentry::Client do ...@@ -36,7 +36,7 @@ describe Sentry::Client do
end end
it 'does not follow redirects' do it 'does not follow redirects' do
expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response error: 302') expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response status code: 302')
expect(redirect_req_stub).to have_been_requested expect(redirect_req_stub).to have_been_requested
expect(redirected_req_stub).not_to have_been_requested expect(redirected_req_stub).not_to have_been_requested
end end
......
...@@ -145,6 +145,24 @@ describe ErrorTracking::ProjectErrorTrackingSetting do ...@@ -145,6 +145,24 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
expect(result).to be_nil expect(result).to be_nil
end end
end end
context 'when sentry client raises exception' do
let(:sentry_client) { spy(:sentry_client) }
before do
synchronous_reactive_cache(subject)
allow(subject).to receive(:sentry_client).and_return(sentry_client)
allow(sentry_client).to receive(:list_issues).with(opts)
.and_raise(Sentry::Client::Error, 'error message')
end
it 'returns error' do
expect(result).to eq(error: 'error message')
expect(subject).to have_received(:sentry_client)
expect(sentry_client).to have_received(:list_issues)
end
end
end end
describe '#list_sentry_projects' do describe '#list_sentry_projects' do
......
...@@ -45,7 +45,23 @@ describe ErrorTracking::ListIssuesService do ...@@ -45,7 +45,23 @@ describe ErrorTracking::ListIssuesService do
it 'result is not ready' do it 'result is not ready' do
expect(result).to eq( expect(result).to eq(
status: :error, http_status: :no_content, message: 'not ready') status: :error, http_status: :no_content, message: 'Not ready. Try again later')
end
end
context 'when list_sentry_issues returns error' do
before do
allow(error_tracking_setting)
.to receive(:list_sentry_issues)
.and_return(error: 'Sentry response status code: 401')
end
it 'returns the error' do
expect(result).to eq(
status: :error,
http_status: :bad_request,
message: 'Sentry response status code: 401'
)
end end
end end
end end
...@@ -58,7 +74,11 @@ describe ErrorTracking::ListIssuesService do ...@@ -58,7 +74,11 @@ describe ErrorTracking::ListIssuesService do
it 'returns error' do it 'returns error' do
result = subject.execute result = subject.execute
expect(result).to include(status: :error, message: 'access denied') expect(result).to include(
status: :error,
message: 'Access denied',
http_status: :unauthorized
)
end end
end end
...@@ -70,7 +90,7 @@ describe ErrorTracking::ListIssuesService do ...@@ -70,7 +90,7 @@ describe ErrorTracking::ListIssuesService do
it 'raises error' do it 'raises error' do
result = subject.execute result = subject.execute
expect(result).to include(status: :error, message: 'not enabled') expect(result).to include(status: :error, message: 'Error Tracking is not enabled')
end end
end end
end end
......
...@@ -53,11 +53,11 @@ describe ErrorTracking::ListProjectsService do ...@@ -53,11 +53,11 @@ describe ErrorTracking::ListProjectsService do
context 'sentry client raises exception' do context 'sentry client raises exception' do
before do before do
expect(error_tracking_setting).to receive(:list_sentry_projects) expect(error_tracking_setting).to receive(:list_sentry_projects)
.and_raise(Sentry::Client::Error, 'Sentry response error: 500') .and_raise(Sentry::Client::Error, 'Sentry response status code: 500')
end end
it 'returns error response' do it 'returns error response' do
expect(result[:message]).to eq('Sentry response error: 500') expect(result[:message]).to eq('Sentry response status code: 500')
expect(result[:http_status]).to eq(:bad_request) expect(result[:http_status]).to eq(:bad_request)
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment