diff --git a/app/controllers/projects/error_tracking_controller.rb b/app/controllers/projects/error_tracking_controller.rb index 88d0755f41f5cc513296eb29c1ad92e46c04317a..9dea6b663ea95c933258e9dacf124f15f43a443a 100644 --- a/app/controllers/projects/error_tracking_controller.rb +++ b/app/controllers/projects/error_tracking_controller.rb @@ -15,6 +15,23 @@ def index end end + def details + respond_to do |format| + format.html + format.json do + render_issue_detail_json + end + end + end + + def stack_trace + respond_to do |format| + format.json do + render_issue_stack_trace_json + end + end + end + def list_projects respond_to do |format| format.json do @@ -29,10 +46,7 @@ def render_index_json service = ErrorTracking::ListIssuesService.new(project, current_user) result = service.execute - unless result[:status] == :success - return render json: { message: result[:message] }, - status: result[:http_status] || :bad_request - end + return if handle_errors(result) render json: { errors: serialize_errors(result[:issues]), @@ -40,6 +54,28 @@ def render_index_json } end + def render_issue_detail_json + service = ErrorTracking::IssueDetailsService.new(project, current_user, issue_details_params) + result = service.execute + + return if handle_errors(result) + + render json: { + error: serialize_detailed_error(result[:issue]) + } + end + + def render_issue_stack_trace_json + service = ErrorTracking::IssueLatestEventService.new(project, current_user, issue_details_params) + result = service.execute + + return if handle_errors(result) + + render json: { + error: serialize_error_event(result[:latest_event]) + } + end + def render_project_list_json service = ErrorTracking::ListProjectsService.new( project, @@ -62,10 +98,21 @@ def render_project_list_json end end + def handle_errors(result) + unless result[:status] == :success + render json: { message: result[:message] }, + status: result[:http_status] || :bad_request + end + end + def list_projects_params params.require(:error_tracking_setting).permit([:api_host, :token]) end + def issue_details_params + params.permit(:issue_id) + end + def set_polling_interval Gitlab::PollingInterval.set_header(response, interval: POLLING_INTERVAL) end @@ -76,6 +123,18 @@ def serialize_errors(errors) .represent(errors) end + def serialize_detailed_error(error) + ErrorTracking::DetailedErrorSerializer + .new(project: project, user: current_user) + .represent(error) + end + + def serialize_error_event(event) + ErrorTracking::ErrorEventSerializer + .new(project: project, user: current_user) + .represent(event) + end + def serialize_projects(projects) ErrorTracking::ProjectSerializer .new(project: project, user: current_user) diff --git a/app/helpers/projects/error_tracking_helper.rb b/app/helpers/projects/error_tracking_helper.rb index fd1222a1dfba1ba54b17bf95586417ad11a47fae..2f5f612ed4c66a37cc8f48b06af5a00393777de2 100644 --- a/app/helpers/projects/error_tracking_helper.rb +++ b/app/helpers/projects/error_tracking_helper.rb @@ -13,4 +13,13 @@ def error_tracking_data(current_user, project) 'illustration-path' => image_path('illustrations/cluster_popover.svg') } end + + def error_details_data(project, issue) + opts = [project, issue, { format: :json }] + + { + 'issue-details-path' => details_namespace_project_error_tracking_index_path(*opts), + 'issue-stack-trace-path' => stack_trace_namespace_project_error_tracking_index_path(*opts) + } + end end diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index 0b4fef5eac12e6595877c3b3db5ebd94cc5aa592..0fa19b1cedce49c722cfab8fe4d3edbea43d58b4 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -87,10 +87,30 @@ def list_sentry_projects { projects: sentry_client.list_projects } end + def issue_details(opts = {}) + with_reactive_cache('issue_details', opts.stringify_keys) do |result| + result + end + end + + def issue_latest_event(opts = {}) + with_reactive_cache('issue_latest_event', opts.stringify_keys) do |result| + result + end + end + def calculate_reactive_cache(request, opts) case request when 'list_issues' { issues: sentry_client.list_issues(**opts.symbolize_keys) } + when 'issue_details' + { + issue: sentry_client.issue_details(**opts.symbolize_keys) + } + when 'issue_latest_event' + { + latest_event: sentry_client.issue_latest_event(**opts.symbolize_keys) + } end rescue Sentry::Client::Error => e { error: e.message, error_type: SENTRY_API_ERROR_TYPE_NON_20X_RESPONSE } diff --git a/app/serializers/error_tracking/detailed_error_entity.rb b/app/serializers/error_tracking/detailed_error_entity.rb new file mode 100644 index 0000000000000000000000000000000000000000..8f08f84aa4115e7e29fdf78ec1818d05b5d31e34 --- /dev/null +++ b/app/serializers/error_tracking/detailed_error_entity.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module ErrorTracking + class DetailedErrorEntity < Grape::Entity + expose :count, + :culprit, + :external_base_url, + :external_url, + :first_release_last_commit, + :first_release_short_version, + :first_seen, + :frequency, + :id, + :last_release_last_commit, + :last_release_short_version, + :last_seen, + :message, + :project_id, + :project_name, + :project_slug, + :short_id, + :status, + :title, + :type, + :user_count + end +end diff --git a/app/serializers/error_tracking/detailed_error_serializer.rb b/app/serializers/error_tracking/detailed_error_serializer.rb new file mode 100644 index 0000000000000000000000000000000000000000..201da16a1ae2cba81aba004044607916572eec70 --- /dev/null +++ b/app/serializers/error_tracking/detailed_error_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module ErrorTracking + class DetailedErrorSerializer < BaseSerializer + entity DetailedErrorEntity + end +end diff --git a/app/serializers/error_tracking/error_event_entity.rb b/app/serializers/error_tracking/error_event_entity.rb new file mode 100644 index 0000000000000000000000000000000000000000..6cf0e6e3ae25e845ff62fbee41fb6e5a394e13f2 --- /dev/null +++ b/app/serializers/error_tracking/error_event_entity.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module ErrorTracking + class ErrorEventEntity < Grape::Entity + expose :issue_id, :date_received, :stack_trace_entries + end +end diff --git a/app/serializers/error_tracking/error_event_serializer.rb b/app/serializers/error_tracking/error_event_serializer.rb new file mode 100644 index 0000000000000000000000000000000000000000..bc4eae1636822adba358caaf1318709d2b1a24fd --- /dev/null +++ b/app/serializers/error_tracking/error_event_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module ErrorTracking + class ErrorEventSerializer < BaseSerializer + entity ErrorEventEntity + end +end diff --git a/app/services/error_tracking/base_service.rb b/app/services/error_tracking/base_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..430d99523323e568f62a17e546ed827f638bab81 --- /dev/null +++ b/app/services/error_tracking/base_service.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module ErrorTracking + class BaseService < ::BaseService + def execute + unauthorized = check_permissions + return unauthorized if unauthorized + + begin + response = fetch + rescue Sentry::Client::Error => e + return error(e.message, :bad_request) + rescue Sentry::Client::MissingKeysError => e + return error(e.message, :internal_server_error) + end + + errors = parse_errors(response) + return errors if errors + + success(parse_response(response)) + end + + private + + def fetch + raise NotImplementedError, + "#{self.class} does not implement #{__method__}" + end + + def parse_response(response) + raise NotImplementedError, + "#{self.class} does not implement #{__method__}" + end + + def check_permissions + return error('Error Tracking is not enabled') unless enabled? + return error('Access denied', :unauthorized) unless can_read? + end + + def parse_errors(response) + return error('Not ready. Try again later', :no_content) unless response + return error(response[:error], http_status_for(response[:error_type])) if response[:error].present? + end + + def http_status_for(error_type) + case error_type + when ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS + :internal_server_error + else + :bad_request + end + end + + def project_error_tracking_setting + project.error_tracking_setting + end + + def enabled? + project_error_tracking_setting&.enabled? + end + + def can_read? + can?(current_user, :read_sentry_issue, project) + end + end +end diff --git a/app/services/error_tracking/issue_details_service.rb b/app/services/error_tracking/issue_details_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..368cd4517fcc4f5922c7a3b06b27783524fc2a8b --- /dev/null +++ b/app/services/error_tracking/issue_details_service.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module ErrorTracking + class IssueDetailsService < ErrorTracking::BaseService + private + + def fetch + project_error_tracking_setting.issue_details(issue_id: params[:issue_id]) + end + + def parse_response(response) + { issue: response[:issue] } + end + end +end diff --git a/app/services/error_tracking/issue_latest_event_service.rb b/app/services/error_tracking/issue_latest_event_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..b6ad8f8028b3c5d8db5f6b1f4d4b5554e303eb01 --- /dev/null +++ b/app/services/error_tracking/issue_latest_event_service.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module ErrorTracking + class IssueLatestEventService < ErrorTracking::BaseService + private + + def fetch + project_error_tracking_setting.issue_latest_event(issue_id: params[:issue_id]) + end + + def parse_response(response) + { latest_event: response[:latest_event] } + end + end +end diff --git a/app/services/error_tracking/list_issues_service.rb b/app/services/error_tracking/list_issues_service.rb index 86ab21fa865d0f3b709baa234483cdf09b8f6019..e09f4345308cd3a6a8403338fae4cdef77e21a06 100644 --- a/app/services/error_tracking/list_issues_service.rb +++ b/app/services/error_tracking/list_issues_service.rb @@ -1,48 +1,24 @@ # frozen_string_literal: true module ErrorTracking - class ListIssuesService < ::BaseService + class ListIssuesService < ErrorTracking::BaseService DEFAULT_ISSUE_STATUS = 'unresolved' DEFAULT_LIMIT = 20 - def execute - return error('Error Tracking is not enabled') unless enabled? - return error('Access denied', :unauthorized) unless can_read? - - result = project_error_tracking_setting - .list_sentry_issues(issue_status: issue_status, limit: limit) - - # our results are not yet ready - unless result - return error('Not ready. Try again later', :no_content) - end + private - if result[:error].present? - return error(result[:error], http_status_from_error_type(result[:error_type])) - end + def fetch + project_error_tracking_setting.list_sentry_issues(issue_status: issue_status, limit: limit) + end - success(issues: result[:issues]) + def parse_response(response) + { issues: response[:issues] } end def external_url project_error_tracking_setting&.sentry_external_url end - private - - def http_status_from_error_type(error_type) - case error_type - when ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS - :internal_server_error - else - :bad_request - end - end - - def project_error_tracking_setting - project.error_tracking_setting - end - def issue_status params[:issue_status] || DEFAULT_ISSUE_STATUS end @@ -50,13 +26,5 @@ def issue_status def limit params[:limit] || DEFAULT_LIMIT end - - def enabled? - project_error_tracking_setting&.enabled? - end - - def can_read? - can?(current_user, :read_sentry_issue, project) - end end end diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb index 92d4ef85ecf67c9c9331e365e524c1f8228792df..09a0b952e84982590b4b49949e3f506fea868b61 100644 --- a/app/services/error_tracking/list_projects_service.rb +++ b/app/services/error_tracking/list_projects_service.rb @@ -1,44 +1,38 @@ # frozen_string_literal: true module ErrorTracking - class ListProjectsService < ::BaseService + class ListProjectsService < ErrorTracking::BaseService def execute - return error('access denied') unless can_read? - - setting = project_error_tracking_setting - - unless setting.valid? - return error(setting.errors.full_messages.join(', '), :bad_request) + unless project_error_tracking_setting.valid? + return error(project_error_tracking_setting.errors.full_messages.join(', '), :bad_request) end - begin - 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]) + super end private - def project_error_tracking_setting - (project.error_tracking_setting || project.build_error_tracking_setting).tap do |setting| - setting.api_url = ErrorTracking::ProjectErrorTrackingSetting.build_api_url_from( - api_host: params[:api_host], - organization_slug: 'org', - project_slug: 'proj' - ) - - setting.token = token(setting) - setting.enabled = true - end + def fetch + project_error_tracking_setting.list_sentry_projects + end + + def parse_response(response) + { projects: response[:projects] } end - def can_read? - can?(current_user, :read_sentry_issue, project) + def project_error_tracking_setting + @project_error_tracking_setting ||= begin + (super || project.build_error_tracking_setting).tap do |setting| + setting.api_url = ErrorTracking::ProjectErrorTrackingSetting.build_api_url_from( + api_host: params[:api_host], + organization_slug: 'org', + project_slug: 'proj' + ) + + setting.token = token(setting) + setting.enabled = true + end + end end def token(setting) diff --git a/app/views/projects/error_tracking/details.html.haml b/app/views/projects/error_tracking/details.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..52e748fa88bc22be31152d0a11e7815f06faba52 --- /dev/null +++ b/app/views/projects/error_tracking/details.html.haml @@ -0,0 +1,3 @@ +- page_title _('Error Details') + +#js-error_tracking{ data: error_details_data(@current_user, @project) } diff --git a/changelogs/unreleased/32464-backend-sentry-error-details.yml b/changelogs/unreleased/32464-backend-sentry-error-details.yml new file mode 100644 index 0000000000000000000000000000000000000000..aa7b44f9d083e38f45decbfa15fcebe5eee33129 --- /dev/null +++ b/changelogs/unreleased/32464-backend-sentry-error-details.yml @@ -0,0 +1,5 @@ +--- +title: API for stack trace & detail view of Sentry error in GitLab +merge_request: 19137 +author: +type: added diff --git a/config/routes/project.rb b/config/routes/project.rb index d49ba20ce843c06914991cabb2114ed19f95e953..380460f4e50180cd34f215ee21446a84cdf907b3 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -613,6 +613,12 @@ resources :error_tracking, only: [:index], controller: :error_tracking do collection do + get ':issue_id/details', + to: 'error_tracking#details', + as: 'details' + get ':issue_id/stack_trace', + to: 'error_tracking#stack_trace', + as: 'stack_trace' post :list_projects end end diff --git a/lib/gitlab/error_tracking/detailed_error.rb b/lib/gitlab/error_tracking/detailed_error.rb new file mode 100644 index 0000000000000000000000000000000000000000..225280a42f4bd9d59d0372f5965fd4a00a4cfda7 --- /dev/null +++ b/lib/gitlab/error_tracking/detailed_error.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Gitlab + module ErrorTracking + class DetailedError + include ActiveModel::Model + + attr_accessor :count, + :culprit, + :external_base_url, + :external_url, + :first_release_last_commit, + :first_release_short_version, + :first_seen, + :frequency, + :id, + :last_release_last_commit, + :last_release_short_version, + :last_seen, + :message, + :project_id, + :project_name, + :project_slug, + :short_id, + :status, + :title, + :type, + :user_count + end + end +end diff --git a/lib/gitlab/error_tracking/error_event.rb b/lib/gitlab/error_tracking/error_event.rb new file mode 100644 index 0000000000000000000000000000000000000000..c6e0d82f8689b1d45ce254639953b9f7fea61875 --- /dev/null +++ b/lib/gitlab/error_tracking/error_event.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Gitlab + module ErrorTracking + class ErrorEvent + include ActiveModel::Model + + attr_accessor :issue_id, :date_received, :stack_trace_entries + end + end +end diff --git a/lib/sentry/client.rb b/lib/sentry/client.rb index 07cca1c8d1e412b9e51ca6cc9f560ca1b7e533b0..f16df0dde4ffaae14d99dfa6090c96c4fed58dfc 100644 --- a/lib/sentry/client.rb +++ b/lib/sentry/client.rb @@ -12,6 +12,18 @@ def initialize(api_url, token) @token = token end + def issue_details(issue_id:) + issue = get_issue(issue_id: issue_id) + + map_to_detailed_error(issue) + end + + def issue_latest_event(issue_id:) + latest_event = get_issue_latest_event(issue_id: issue_id) + + map_to_event(latest_event) + end + def list_issues(issue_status:, limit:) issues = get_issues(issue_status: issue_status, limit: limit) @@ -61,6 +73,14 @@ def get_issues(issue_status:, limit:) }) end + def get_issue(issue_id:) + http_get(issue_api_url(issue_id)) + end + + def get_issue_latest_event(issue_id:) + http_get(issue_latest_event_api_url(issue_id)) + end + def get_projects http_get(projects_api_url) end @@ -102,6 +122,20 @@ def projects_api_url projects_url end + def issue_api_url(issue_id) + issue_url = URI(@url) + issue_url.path = "/api/0/issues/#{issue_id}/" + + issue_url + end + + def issue_latest_event_api_url(issue_id) + latest_event_url = URI(@url) + latest_event_url.path = "/api/0/issues/#{issue_id}/events/latest/" + + latest_event_url + end + def issues_api_url issues_url = URI(@url + '/issues/') issues_url.path.squeeze!('/') @@ -119,38 +153,87 @@ def map_to_projects(projects) def issue_url(id) issues_url = @url + "/issues/#{id}" - issues_url = ErrorTracking::ProjectErrorTrackingSetting.extract_sentry_external_url(issues_url) - uri = URI(issues_url) + parse_sentry_url(issues_url) + end + + def project_url + parse_sentry_url(@url) + end + + def parse_sentry_url(api_url) + url = ErrorTracking::ProjectErrorTrackingSetting.extract_sentry_external_url(api_url) + + uri = URI(url) uri.path.squeeze!('/') + # Remove trailing spaces + uri = uri.to_s.gsub(/\/\z/, '') - uri.to_s + uri end - def map_to_error(issue) - id = issue.fetch('id') + def map_to_event(event) + stack_trace = parse_stack_trace(event) + + Gitlab::ErrorTracking::ErrorEvent.new( + issue_id: event.dig('groupID'), + date_received: event.dig('dateReceived'), + stack_trace_entries: stack_trace + ) + end - count = issue.fetch('count', nil) + def parse_stack_trace(event) + exception_entry = event.dig('entries')&.detect { |h| h['type'] == 'exception' } + return unless exception_entry - frequency = issue.dig('stats', '24h') - message = issue.dig('metadata', 'value') + exception_values = exception_entry.dig('data', 'values') + stack_trace_entry = exception_values&.detect { |h| h['stacktrace'].present? } + return unless stack_trace_entry - external_url = issue_url(id) + stack_trace_entry.dig('stacktrace', 'frames') + end + + def map_to_detailed_error(issue) + Gitlab::ErrorTracking::DetailedError.new( + id: issue.fetch('id'), + first_seen: issue.fetch('firstSeen', nil), + last_seen: issue.fetch('lastSeen', nil), + title: issue.fetch('title', nil), + type: issue.fetch('type', nil), + user_count: issue.fetch('userCount', nil), + count: issue.fetch('count', nil), + message: issue.dig('metadata', 'value'), + culprit: issue.fetch('culprit', nil), + external_url: issue_url(issue.fetch('id')), + external_base_url: project_url, + short_id: issue.fetch('shortId', nil), + status: issue.fetch('status', nil), + frequency: issue.dig('stats', '24h'), + project_id: issue.dig('project', 'id'), + project_name: issue.dig('project', 'name'), + project_slug: issue.dig('project', 'slug'), + first_release_last_commit: issue.dig('firstRelease', 'lastCommit'), + last_release_last_commit: issue.dig('lastRelease', 'lastCommit'), + first_release_short_version: issue.dig('firstRelease', 'shortVersion'), + last_release_short_version: issue.dig('lastRelease', 'shortVersion') + ) + end + def map_to_error(issue) Gitlab::ErrorTracking::Error.new( - id: id, + id: issue.fetch('id'), first_seen: issue.fetch('firstSeen', nil), last_seen: issue.fetch('lastSeen', nil), title: issue.fetch('title', nil), type: issue.fetch('type', nil), user_count: issue.fetch('userCount', nil), - count: count, - message: message, + count: issue.fetch('count', nil), + message: issue.dig('metadata', 'value'), culprit: issue.fetch('culprit', nil), - external_url: external_url, + external_url: issue_url(issue.fetch('id')), short_id: issue.fetch('shortId', nil), status: issue.fetch('status', nil), - frequency: frequency, + frequency: issue.dig('stats', '24h'), project_id: issue.dig('project', 'id'), project_name: issue.dig('project', 'name'), project_slug: issue.dig('project', 'slug') diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c051d06f125c4f08ddaf4b623e1aaab8be3651c2..367982174feed5652c87678fbfa878d7e8550a5d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6583,6 +6583,9 @@ msgstr "" msgid "Error" msgstr "" +msgid "Error Details" +msgstr "" + msgid "Error Tracking" msgstr "" diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index 31868f5f7173167cb2b633b0044f6da2682089ea..8155d6ddafe77ec1c216d5164a7b9eeb03910bc1 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -46,17 +46,6 @@ end describe 'format json' do - shared_examples 'no data' do - it 'returns no data' do - get :index, params: project_params(format: :json) - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('error_tracking/index') - expect(json_response['external_url']).to be_nil - expect(json_response['errors']).to eq([]) - end - end - let(:list_issues_service) { spy(:list_issues_service) } let(:external_url) { 'http://example.com' } @@ -66,6 +55,19 @@ .and_return(list_issues_service) end + context 'no data' do + before do + expect(list_issues_service).to receive(:execute) + .and_return(status: :error, http_status: :no_content) + end + + it 'returns no data' do + get :index, params: project_params(format: :json) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + context 'service result is successful' do before do expect(list_issues_service).to receive(:execute) @@ -232,8 +234,186 @@ def list_projects_params(opts = {}) end end + describe 'GET #issue_details' do + let_it_be(:issue_id) { 1234 } + + let(:issue_details_service) { spy(:issue_details_service) } + + let(:permitted_params) do + ActionController::Parameters.new( + { issue_id: issue_id.to_s } + ).permit! + end + + before do + expect(ErrorTracking::IssueDetailsService) + .to receive(:new).with(project, user, permitted_params) + .and_return(issue_details_service) + end + + describe 'format json' do + context 'no data' do + before do + expect(issue_details_service).to receive(:execute) + .and_return(status: :error, http_status: :no_content) + end + + it 'returns no data' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'service result is successful' do + before do + expect(issue_details_service).to receive(:execute) + .and_return(status: :success, issue: error) + end + + let(:error) { build(:detailed_error_tracking_error) } + + it 'returns an error' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/issue_detailed') + expect(json_response['error']).to eq(error.as_json) + end + end + + context 'service result is erroneous' do + let(:error_message) { 'error message' } + + context 'without http_status' do + before do + expect(issue_details_service).to receive(:execute) + .and_return(status: :error, message: error_message) + end + + it 'returns 400 with message' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(error_message) + end + end + + context 'with explicit http_status' do + let(:http_status) { :no_content } + + before do + expect(issue_details_service).to receive(:execute).and_return( + status: :error, + message: error_message, + http_status: http_status + ) + end + + it 'returns http_status with message' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['message']).to eq(error_message) + end + end + end + end + end + + describe 'GET #stack_trace' do + let_it_be(:issue_id) { 1234 } + + let(:issue_stack_trace_service) { spy(:issue_stack_trace_service) } + + let(:permitted_params) do + ActionController::Parameters.new( + { issue_id: issue_id.to_s } + ).permit! + end + + before do + expect(ErrorTracking::IssueLatestEventService) + .to receive(:new).with(project, user, permitted_params) + .and_return(issue_stack_trace_service) + end + + describe 'format json' do + context 'awaiting data' do + before do + expect(issue_stack_trace_service).to receive(:execute) + .and_return(status: :error, http_status: :no_content) + end + + it 'returns no data' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'service result is successful' do + before do + expect(issue_stack_trace_service).to receive(:execute) + .and_return(status: :success, latest_event: error_event) + end + + let(:error_event) { build(:error_tracking_error_event) } + + it 'returns an error' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/issue_stack_trace') + expect(json_response['error']).to eq(error_event.as_json) + end + end + + context 'service result is erroneous' do + let(:error_message) { 'error message' } + + context 'without http_status' do + before do + expect(issue_stack_trace_service).to receive(:execute) + .and_return(status: :error, message: error_message) + end + + it 'returns 400 with message' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(error_message) + end + end + + context 'with explicit http_status' do + let(:http_status) { :no_content } + + before do + expect(issue_stack_trace_service).to receive(:execute).and_return( + status: :error, + message: error_message, + http_status: http_status + ) + end + + it 'returns http_status with message' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['message']).to eq(error_message) + end + end + end + end + end + private + def issue_params(opts = {}) + project_params.reverse_merge(opts) + end + def project_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project) end diff --git a/spec/factories/error_tracking/detailed_error.rb b/spec/factories/error_tracking/detailed_error.rb new file mode 100644 index 0000000000000000000000000000000000000000..cf7de2ece964b04704fa6e0c3cbfebfb7faf03b8 --- /dev/null +++ b/spec/factories/error_tracking/detailed_error.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :detailed_error_tracking_error, class: Gitlab::ErrorTracking::DetailedError do + id { 'id' } + title { 'title' } + type { 'error' } + user_count { 1 } + count { 2 } + first_seen { Time.now } + last_seen { Time.now } + message { 'message' } + culprit { 'culprit' } + external_url { 'http://example.com/id' } + external_base_url { 'http://example.com' } + project_id { 'project1' } + project_name { 'project name' } + project_slug { 'project_name' } + short_id { 'ID' } + status { 'unresolved' } + frequency { [] } + first_release_last_commit { '68c914da9' } + last_release_last_commit { '9ad419c86' } + first_release_short_version { 'abc123' } + last_release_short_version { 'abc123' } + + skip_create + end +end diff --git a/spec/factories/error_tracking/error_event.rb b/spec/factories/error_tracking/error_event.rb new file mode 100644 index 0000000000000000000000000000000000000000..44c127e7bf57cff1b3a7d8ac6eb0ea4ef015953f --- /dev/null +++ b/spec/factories/error_tracking/error_event.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :error_tracking_error_event, class: Gitlab::ErrorTracking::ErrorEvent do + issue_id { 'id' } + date_received { Time.now.iso8601 } + stack_trace_entries do + { + 'stacktrace' => + { + 'frames' => [{ 'file' => 'test.rb' }] + } + } + end + + skip_create + end +end diff --git a/spec/fixtures/api/schemas/error_tracking/error.json b/spec/fixtures/api/schemas/error_tracking/error.json index df2c02d7d5da01e291a1d0ae8a923893b3e92913..3f65105681e4d2c42ab4c5736f12aed53de46f2d 100644 --- a/spec/fixtures/api/schemas/error_tracking/error.json +++ b/spec/fixtures/api/schemas/error_tracking/error.json @@ -4,7 +4,14 @@ "external_url", "last_seen", "message", - "type" + "type", + "title", + "project_id", + "project_name", + "project_slug", + "short_id", + "status", + "frequency" ], "properties" : { "id": { "type": "string"}, @@ -15,7 +22,14 @@ "culprit": { "type": "string" }, "count": { "type": "integer"}, "external_url": { "type": "string" }, - "user_count": { "type": "integer"} + "user_count": { "type": "integer"}, + "title": { "type": "string"}, + "project_id": { "type": "string"}, + "project_name": { "type": "string"}, + "project_slug": { "type": "string"}, + "short_id": { "type": "string"}, + "status": { "type": "string"}, + "frequency": { "type": "array"} }, - "additionalProperties": true + "additionalProperties": false } diff --git a/spec/fixtures/api/schemas/error_tracking/error_detailed.json b/spec/fixtures/api/schemas/error_tracking/error_detailed.json new file mode 100644 index 0000000000000000000000000000000000000000..40d6773f0e630cac38b4da4476ce639a25201dd2 --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/error_detailed.json @@ -0,0 +1,45 @@ +{ + "type": "object", + "required" : [ + "external_url", + "external_base_url", + "last_seen", + "message", + "type", + "title", + "project_id", + "project_name", + "project_slug", + "short_id", + "status", + "frequency", + "first_release_last_commit", + "last_release_last_commit", + "first_release_short_version", + "last_release_short_version" + ], + "properties" : { + "id": { "type": "string"}, + "first_seen": { "type": "string", "format": "date-time" }, + "last_seen": { "type": "string", "format": "date-time" }, + "type": { "type": "string" }, + "message": { "type": "string" }, + "culprit": { "type": "string" }, + "count": { "type": "integer"}, + "external_url": { "type": "string" }, + "external_base_url": { "type": "string" }, + "user_count": { "type": "integer"}, + "title": { "type": "string"}, + "project_id": { "type": "string"}, + "project_name": { "type": "string"}, + "project_slug": { "type": "string"}, + "short_id": { "type": "string"}, + "status": { "type": "string"}, + "frequency": { "type": "array"}, + "first_release_last_commit": { "type": ["string", "null"] }, + "last_release_last_commit": { "type": ["string", "null"] }, + "first_release_short_version": { "type": ["string", "null"] }, + "last_release_short_version": { "type": ["string", "null"] } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/error_tracking/error_stack_trace.json b/spec/fixtures/api/schemas/error_tracking/error_stack_trace.json new file mode 100644 index 0000000000000000000000000000000000000000..a684dd0496a4ae1774f065d3774e79150c9bd4ee --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/error_stack_trace.json @@ -0,0 +1,14 @@ +{ + "type": "object", + "required": [ + "issue_id", + "stack_trace_entries", + "date_received" + ], + "properties": { + "issue_id": { "type": ["string", "integer"] }, + "stack_trace_entries": { "type": "object" }, + "date_received": { "type": "string" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/error_tracking/issue_detailed.json b/spec/fixtures/api/schemas/error_tracking/issue_detailed.json new file mode 100644 index 0000000000000000000000000000000000000000..b5adea6fc623aebbaef86be03246fd52ade2dc52 --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/issue_detailed.json @@ -0,0 +1,11 @@ +{ + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { "$ref": "error_detailed.json" } + }, + "additionalProperties": false +} + diff --git a/spec/fixtures/api/schemas/error_tracking/issue_stack_trace.json b/spec/fixtures/api/schemas/error_tracking/issue_stack_trace.json new file mode 100644 index 0000000000000000000000000000000000000000..7ec1ae63609a80d52feaee6738541257f5e926d4 --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/issue_stack_trace.json @@ -0,0 +1,11 @@ +{ + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { "$ref": "error_stack_trace.json" } + }, + "additionalProperties": false +} + diff --git a/spec/services/error_tracking/issue_details_service_spec.rb b/spec/services/error_tracking/issue_details_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..4d5505bb5a912e515e3c9116fd0eef0ea8d32b84 --- /dev/null +++ b/spec/services/error_tracking/issue_details_service_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ErrorTracking::IssueDetailsService do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:token) { 'test-token' } + let(:result) { subject.execute } + + let(:error_tracking_setting) do + create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project) + end + + subject { described_class.new(project, user) } + + before do + expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting) + + project.add_reporter(user) + end + + describe '#execute' do + context 'with authorized user' do + context 'when issue_details returns a detailed error' do + let(:detailed_error) { build(:detailed_error_tracking_error) } + + before do + expect(error_tracking_setting) + .to receive(:issue_details).and_return(issue: detailed_error) + end + + it 'returns the detailed error' do + expect(result).to eq(status: :success, issue: detailed_error) + end + end + + include_examples 'error tracking service data not ready', :issue_details + include_examples 'error tracking service sentry error handling', :issue_details + include_examples 'error tracking service http status handling', :issue_details + end + + include_examples 'error tracking service unauthorized user' + include_examples 'error tracking service disabled' + end +end diff --git a/spec/services/error_tracking/issue_latest_event_service_spec.rb b/spec/services/error_tracking/issue_latest_event_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..cda15042814d9e88613c9468bc8c8f799026f171 --- /dev/null +++ b/spec/services/error_tracking/issue_latest_event_service_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ErrorTracking::IssueLatestEventService do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:token) { 'test-token' } + let(:result) { subject.execute } + + let(:error_tracking_setting) do + create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project) + end + + subject { described_class.new(project, user) } + + before do + expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting) + + project.add_reporter(user) + end + + describe '#execute' do + context 'with authorized user' do + context 'when issue_latest_event returns an error event' do + let(:error_event) { build(:error_tracking_error_event) } + + before do + expect(error_tracking_setting) + .to receive(:issue_latest_event).and_return(latest_event: error_event) + end + + it 'returns the error event' do + expect(result).to eq(status: :success, latest_event: error_event) + end + end + + include_examples 'error tracking service data not ready', :issue_latest_event + include_examples 'error tracking service sentry error handling', :issue_latest_event + include_examples 'error tracking service http status handling', :issue_latest_event + end + + include_examples 'error tracking service unauthorized user' + include_examples 'error tracking service disabled' + end +end diff --git a/spec/services/error_tracking/list_issues_service_spec.rb b/spec/services/error_tracking/list_issues_service_spec.rb index 3a8f3069911db95e659fb2a93f57b08564fbe598..fce790f708df7d61841b994862f7823cdbd519ef 100644 --- a/spec/services/error_tracking/list_issues_service_spec.rb +++ b/spec/services/error_tracking/list_issues_service_spec.rb @@ -37,93 +37,12 @@ end end - context 'when list_sentry_issues returns nil' do - before do - expect(error_tracking_setting) - .to receive(:list_sentry_issues).and_return(nil) - end - - it 'result is not ready' do - expect(result).to eq( - 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', - error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_NON_20X_RESPONSE - ) - end - - it 'returns the error' do - expect(result).to eq( - status: :error, - http_status: :bad_request, - message: 'Sentry response status code: 401' - ) - end - end - - context 'when list_sentry_issues returns error with http_status' do - before do - allow(error_tracking_setting) - .to receive(:list_sentry_issues) - .and_return( - error: 'Sentry API response is missing keys. key not found: "id"', - error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS - ) - end - - it 'returns the error with correct http_status' do - expect(result).to eq( - status: :error, - http_status: :internal_server_error, - message: 'Sentry API response is missing keys. key not found: "id"' - ) - end - end + include_examples 'error tracking service data not ready', :list_sentry_issues + include_examples 'error tracking service sentry error handling', :list_sentry_issues + include_examples 'error tracking service http status handling', :list_sentry_issues end - context 'with unauthorized user' do - let(:unauthorized_user) { create(:user) } - - subject { described_class.new(project, unauthorized_user) } - - it 'returns error' do - result = subject.execute - - expect(result).to include( - status: :error, - message: 'Access denied', - http_status: :unauthorized - ) - end - end - - context 'with error tracking disabled' do - before do - error_tracking_setting.enabled = false - end - - it 'raises error' do - result = subject.execute - - expect(result).to include(status: :error, message: 'Error Tracking is not enabled') - end - end - end - - describe '#sentry_external_url' do - let(:external_url) { 'https://sentrytest.gitlab.com/sentry-org/sentry-project' } - - it 'calls ErrorTracking::ProjectErrorTrackingSetting' do - expect(error_tracking_setting).to receive(:sentry_external_url).and_call_original - - subject.external_url - end + include_examples 'error tracking service unauthorized user' + include_examples 'error tracking service disabled' 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 a272a604184e14de998f6814cdb3be2753c54dfe..cd4b835e0974aa56a8fc277f01e499d82e3d381d 100644 --- a/spec/services/error_tracking/list_projects_service_spec.rb +++ b/spec/services/error_tracking/list_projects_service_spec.rb @@ -127,7 +127,7 @@ end it 'returns error' do - expect(result).to include(status: :error, message: 'access denied') + expect(result).to include(status: :error, message: 'Access denied', http_status: :unauthorized) end end diff --git a/spec/support/shared_examples/services/error_tracking_service_shared_examples.rb b/spec/support/shared_examples/services/error_tracking_service_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..83c6d89e56089672325cfa6aed7811e78985dfa9 --- /dev/null +++ b/spec/support/shared_examples/services/error_tracking_service_shared_examples.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +shared_examples 'error tracking service data not ready' do |service_call| + context "when #{service_call} returns nil" do + before do + expect(error_tracking_setting) + .to receive(service_call).and_return(nil) + end + + it 'result is not ready' do + expect(result).to eq( + status: :error, http_status: :no_content, message: 'Not ready. Try again later') + end + end +end + +shared_examples 'error tracking service sentry error handling' do |service_call| + context "when #{service_call} returns error" do + before do + allow(error_tracking_setting) + .to receive(service_call) + .and_return( + error: 'Sentry response status code: 401', + error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_NON_20X_RESPONSE + ) + 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 + +shared_examples 'error tracking service http status handling' do |service_call| + context "when #{service_call} returns error with http_status" do + before do + allow(error_tracking_setting) + .to receive(service_call) + .and_return( + error: 'Sentry API response is missing keys. key not found: "id"', + error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS + ) + end + + it 'returns the error with correct http_status' do + expect(result).to eq( + status: :error, + http_status: :internal_server_error, + message: 'Sentry API response is missing keys. key not found: "id"' + ) + end + end +end + +shared_examples 'error tracking service unauthorized user' do + context 'with unauthorized user' do + let(:unauthorized_user) { create(:user) } + + subject { described_class.new(project, unauthorized_user) } + + it 'returns error' do + result = subject.execute + + expect(result).to include( + status: :error, + message: 'Access denied', + http_status: :unauthorized + ) + end + end +end + +shared_examples 'error tracking service disabled' do + context 'with error tracking disabled' do + before do + error_tracking_setting.enabled = false + end + + it 'raises error' do + result = subject.execute + + expect(result).to include(status: :error, message: 'Error Tracking is not enabled') + end + end +end