Commit 8f254454 authored by Sean Arnold's avatar Sean Arnold Committed by Robert Speicher

Add Error tracking setting update_issue

- Error tracking setting method
- Sentry client code
- Refactor error tracking setting error handling
parent b7a76153
# frozen_string_literal: true # frozen_string_literal: true
class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseController class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseController
respond_to :json
before_action :authorize_read_sentry_issue! before_action :authorize_read_sentry_issue!
before_action :set_issue_id, only: :details before_action :set_issue_id, only: :details
...@@ -24,6 +26,17 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle ...@@ -24,6 +26,17 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle
end end
end end
def update
service = ErrorTracking::IssueUpdateService.new(project, current_user, issue_update_params)
result = service.execute
return if handle_errors(result)
render json: {
result: result
}
end
private private
def render_index_json def render_index_json
...@@ -65,6 +78,10 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle ...@@ -65,6 +78,10 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle
params.permit(:search_term, :sort, :cursor) params.permit(:search_term, :sort, :cursor)
end end
def issue_update_params
params.permit(:issue_id, :status)
end
def issue_details_params def issue_details_params
params.permit(:issue_id) params.permit(:issue_id)
end end
......
...@@ -20,6 +20,7 @@ module Projects::ErrorTrackingHelper ...@@ -20,6 +20,7 @@ module Projects::ErrorTrackingHelper
{ {
'project-issues-path' => project_issues_path(project), 'project-issues-path' => project_issues_path(project),
'issue-details-path' => details_project_error_tracking_index_path(*opts), 'issue-details-path' => details_project_error_tracking_index_path(*opts),
'issue-update-path' => update_project_error_tracking_index_path(*opts),
'issue-stack-trace-path' => stack_trace_project_error_tracking_index_path(*opts) 'issue-stack-trace-path' => stack_trace_project_error_tracking_index_path(*opts)
} }
end end
......
...@@ -101,30 +101,27 @@ module ErrorTracking ...@@ -101,30 +101,27 @@ module ErrorTracking
end end
end end
def update_issue(opts = {} )
handle_exceptions do
{ updated: sentry_client.update_issue(opts) }
end
end
def calculate_reactive_cache(request, opts) def calculate_reactive_cache(request, opts)
case request handle_exceptions do
when 'list_issues' case request
sentry_client.list_issues(**opts.symbolize_keys) when 'list_issues'
when 'issue_details' sentry_client.list_issues(**opts.symbolize_keys)
{ when 'issue_details'
issue: sentry_client.issue_details(**opts.symbolize_keys) {
} issue: sentry_client.issue_details(**opts.symbolize_keys)
when 'issue_latest_event' }
{ when 'issue_latest_event'
latest_event: sentry_client.issue_latest_event(**opts.symbolize_keys) {
} latest_event: sentry_client.issue_latest_event(**opts.symbolize_keys)
}
end
end end
rescue Sentry::Client::Error => e
{ error: e.message, error_type: SENTRY_API_ERROR_TYPE_NON_20X_RESPONSE }
rescue Sentry::Client::MissingKeysError => e
{ error: e.message, error_type: SENTRY_API_ERROR_TYPE_MISSING_KEYS }
rescue Sentry::Client::ResponseInvalidSizeError => e
{ error: e.message, error_type: SENTRY_API_ERROR_INVALID_SIZE }
rescue Sentry::Client::BadRequestError => e
{ error: e.message, error_type: SENTRY_API_ERROR_TYPE_BAD_REQUEST }
rescue StandardError => e
Gitlab::ErrorTracking.track_exception(e)
{ error: 'Unexpected Error' }
end end
# http://HOST/api/0/projects/ORG/PROJECT # http://HOST/api/0/projects/ORG/PROJECT
...@@ -143,6 +140,21 @@ module ErrorTracking ...@@ -143,6 +140,21 @@ module ErrorTracking
private private
def handle_exceptions
yield
rescue Sentry::Client::Error => e
{ error: e.message, error_type: SENTRY_API_ERROR_TYPE_NON_20X_RESPONSE }
rescue Sentry::Client::MissingKeysError => e
{ error: e.message, error_type: SENTRY_API_ERROR_TYPE_MISSING_KEYS }
rescue Sentry::Client::ResponseInvalidSizeError => e
{ error: e.message, error_type: SENTRY_API_ERROR_INVALID_SIZE }
rescue Sentry::Client::BadRequestError => e
{ error: e.message, error_type: SENTRY_API_ERROR_TYPE_BAD_REQUEST }
rescue StandardError => e
Gitlab::ErrorTracking.track_exception(e)
{ error: 'Unexpected Error' }
end
def project_name_from_slug def project_name_from_slug
@project_name_from_slug ||= project_slug_from_api_url&.titleize @project_name_from_slug ||= project_slug_from_api_url&.titleize
end end
......
# frozen_string_literal: true
module ErrorTracking
class IssueUpdateService < ErrorTracking::BaseService
private
def fetch
project_error_tracking_setting.update_issue(
issue_id: params[:issue_id],
params: update_params
)
end
def update_params
params.except(:issue_id)
end
def parse_response(response)
{ updated: response[:updated].present? }
end
end
end
---
title: Add internal API to update Sentry error status
merge_request: 22454
author:
type: added
...@@ -275,6 +275,9 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do ...@@ -275,6 +275,9 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
get ':issue_id/stack_trace', get ':issue_id/stack_trace',
to: 'error_tracking/stack_traces#index', to: 'error_tracking/stack_traces#index',
as: 'stack_trace' as: 'stack_trace'
put ':issue_id',
to: 'error_tracking#update',
as: 'update'
end end
end end
......
...@@ -43,7 +43,7 @@ module Sentry ...@@ -43,7 +43,7 @@ module Sentry
def http_put(url, params = {}) def http_put(url, params = {})
http_request do http_request do
Gitlab::HTTP.put(url, **request_params.merge({ body: params })) Gitlab::HTTP.put(url, **request_params.merge(body: params))
end end
end end
......
...@@ -34,6 +34,10 @@ module Sentry ...@@ -34,6 +34,10 @@ module Sentry
map_to_detailed_error(issue) map_to_detailed_error(issue)
end end
def update_issue(issue_id:, params:)
http_put(issue_api_url(issue_id), params)[:body]
end
private private
def get_issues(**keyword_args) def get_issues(**keyword_args)
...@@ -71,10 +75,6 @@ module Sentry ...@@ -71,10 +75,6 @@ module Sentry
http_get(issue_api_url(issue_id))[:body] http_get(issue_api_url(issue_id))[:body]
end end
def update_issue(issue_id:, params:)
http_put(issue_api_url(issue_id), params)[:body]
end
def issues_api_url def issues_api_url
issues_url = URI("#{url}/issues/") issues_url = URI("#{url}/issues/")
issues_url.path.squeeze!('/') issues_url.path.squeeze!('/')
......
...@@ -271,6 +271,58 @@ describe Projects::ErrorTrackingController do ...@@ -271,6 +271,58 @@ describe Projects::ErrorTrackingController do
end end
end end
describe 'PUT #update' do
let(:issue_id) { 1234 }
let(:issue_update_service) { spy(:issue_update_service) }
let(:permitted_params) do
ActionController::Parameters.new(
{ issue_id: issue_id.to_s, status: 'resolved' }
).permit!
end
subject(:update_issue) do
put :update, params: issue_params(issue_id: issue_id, status: 'resolved', format: :json)
end
before do
expect(ErrorTracking::IssueUpdateService)
.to receive(:new).with(project, user, permitted_params)
.and_return(issue_update_service)
end
describe 'format json' do
context 'update result is successful' do
before do
expect(issue_update_service).to receive(:execute)
.and_return(status: :success, updated: true)
update_issue
end
it 'returns a success' do
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('error_tracking/update_issue')
end
end
context 'update result is erroneous' do
let(:error_message) { 'error message' }
before do
expect(issue_update_service).to receive(:execute)
.and_return(status: :error, message: error_message)
update_issue
end
it 'returns 400 with message' do
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']).to eq(error_message)
end
end
end
end
private private
def issue_params(opts = {}) def issue_params(opts = {})
......
{
"type": "object",
"required" : [
"result"
],
"properties" : {
"result": {
"type": "object",
"properties": {
"status": { "type": "string" },
"updated": { "type": "boolean" }
}
}
},
"additionalProperties": false
}
...@@ -6,7 +6,9 @@ describe Sentry::Client::Issue do ...@@ -6,7 +6,9 @@ describe Sentry::Client::Issue do
include SentryClientHelpers include SentryClientHelpers
let(:token) { 'test-token' } let(:token) { 'test-token' }
let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0' }
let(:client) { Sentry::Client.new(sentry_url, token) } let(:client) { Sentry::Client.new(sentry_url, token) }
let(:issue_id) { 503504 }
describe '#list_issues' do describe '#list_issues' do
shared_examples 'issues have correct return type' do |klass| shared_examples 'issues have correct return type' do |klass|
...@@ -225,15 +227,11 @@ describe Sentry::Client::Issue do ...@@ -225,15 +227,11 @@ describe Sentry::Client::Issue do
) )
end end
let(:issue_id) { 503504 }
let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0' }
let(:sentry_request_url) { "#{sentry_url}/issues/#{issue_id}/" } let(:sentry_request_url) { "#{sentry_url}/issues/#{issue_id}/" }
let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: issue_sample_response) } let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: issue_sample_response) }
subject { client.issue_details(issue_id: issue_id) } subject { client.issue_details(issue_id: issue_id) }
it_behaves_like 'calls sentry api'
it 'escapes issue ID' do it 'escapes issue ID' do
allow(CGI).to receive(:escape).and_call_original allow(CGI).to receive(:escape).and_call_original
...@@ -290,4 +288,41 @@ describe Sentry::Client::Issue do ...@@ -290,4 +288,41 @@ describe Sentry::Client::Issue do
end end
end end
end end
describe '#update_issue' do
let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0' }
let(:sentry_request_url) { "#{sentry_url}/issues/#{issue_id}/" }
before do
stub_sentry_request(sentry_request_url, :put)
end
let(:params) do
{
status: 'resolved'
}
end
subject { client.update_issue(issue_id: issue_id, params: params) }
it_behaves_like 'calls sentry api' do
let(:sentry_api_request) { stub_sentry_request(sentry_request_url, :put) }
end
it 'returns a truthy result' do
expect(subject).to be_truthy
end
context 'error encountered' do
let(:error) { StandardError.new('error') }
before do
allow(client).to receive(:update_issue).and_raise(error)
end
it 'raises the error' do
expect { subject }.to raise_error(error)
end
end
end
end end
...@@ -210,6 +210,40 @@ describe ErrorTracking::ProjectErrorTrackingSetting do ...@@ -210,6 +210,40 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
end end
end end
describe '#update_issue' do
let(:opts) do
{ status: 'resolved' }
end
let(:result) do
subject.update_issue(**opts)
end
let(:sentry_client) { spy(:sentry_client) }
context 'successful call to sentry' do
before do
allow(subject).to receive(:sentry_client).and_return(sentry_client)
allow(sentry_client).to receive(:update_issue).with(opts).and_return(true)
end
it 'returns the successful response' do
expect(result).to eq(updated: true)
end
end
context 'sentry raises an error' do
before do
allow(subject).to receive(:sentry_client).and_return(sentry_client)
allow(sentry_client).to receive(:update_issue).with(opts).and_raise(StandardError)
end
it 'returns the successful response' do
expect(result).to eq(error: 'Unexpected Error')
end
end
end
context 'slugs' do context 'slugs' do
shared_examples_for 'slug from api_url' do |method, slug| shared_examples_for 'slug from api_url' do |method, slug|
context 'when api_url is correct' do context 'when api_url is correct' do
......
...@@ -3,8 +3,8 @@ ...@@ -3,8 +3,8 @@
module SentryClientHelpers module SentryClientHelpers
private private
def stub_sentry_request(url, body: {}, status: 200, headers: {}) def stub_sentry_request(url, http_method = :get, body: {}, status: 200, headers: {})
stub_request(:get, url) stub_request(http_method, url)
.to_return( .to_return(
status: status, status: status,
headers: { 'Content-Type' => 'application/json' }.merge(headers), headers: { 'Content-Type' => 'application/json' }.merge(headers),
......
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