Merge branch '39825-resolve-sentry-error-backend' into 'master'

Endpoint to update Sentry error status in Sentry

See merge request gitlab-org/gitlab!22454

app/controllers/projects/error_tracking_controller.rb

 # frozen_string_literal: true
 
 class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseController
+  respond_to :json
+
   before_action :authorize_read_sentry_issue!
   before_action :set_issue_id, only: :details
 
@@ -24,6 +26,17 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle
     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
 
   def render_index_json
@@ -65,6 +78,10 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle
     params.permit(:search_term, :sort, :cursor)
   end
 
+  def issue_update_params
+    params.permit(:issue_id, :status)
+  end
+
   def issue_details_params
     params.permit(:issue_id)
   end

app/helpers/projects/error_tracking_helper.rb

@@ -20,6 +20,7 @@ module Projects::ErrorTrackingHelper
     {
       'project-issues-path' => project_issues_path(project),
       '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)
     }
   end

app/models/error_tracking/project_error_tracking_setting.rb

@@ -101,30 +101,27 @@ module ErrorTracking
       end
     end
 
+    def update_issue(opts = {} )
+      handle_exceptions do
+        { updated: sentry_client.update_issue(opts) }
+      end
+    end
+
     def calculate_reactive_cache(request, opts)
-      case request
-      when 'list_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)
-        }
+      handle_exceptions do
+        case request
+        when 'list_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
       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
 
     # http://HOST/api/0/projects/ORG/PROJECT
@@ -143,6 +140,21 @@ module ErrorTracking
 
     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
       @project_name_from_slug ||= project_slug_from_api_url&.titleize
     end

app/services/error_tracking/issue_update_service.rb

+# 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

changelogs/unreleased/39825-resolve-sentry-error-backend.yml

+---
+title: Add internal API to update Sentry error status
+merge_request: 22454
+author:
+type: added

config/routes/project.rb

@@ -275,6 +275,9 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
             get ':issue_id/stack_trace',
               to: 'error_tracking/stack_traces#index',
               as: 'stack_trace'
+            put ':issue_id',
+              to: 'error_tracking#update',
+              as: 'update'
           end
         end
 

lib/sentry/client.rb

@@ -43,7 +43,7 @@ module Sentry
 
     def http_put(url, params = {})
       http_request do
-        Gitlab::HTTP.put(url, **request_params.merge({ body: params }))
+        Gitlab::HTTP.put(url, **request_params.merge(body: params))
       end
     end
 

lib/sentry/client/issue.rb

@@ -34,6 +34,10 @@ module Sentry
         map_to_detailed_error(issue)
       end
 
+      def update_issue(issue_id:, params:)
+        http_put(issue_api_url(issue_id), params)[:body]
+      end
+
       private
 
       def get_issues(**keyword_args)
@@ -71,10 +75,6 @@ module Sentry
         http_get(issue_api_url(issue_id))[:body]
       end
 
-      def update_issue(issue_id:, params:)
-        http_put(issue_api_url(issue_id), params)[:body]
-      end
-
       def issues_api_url
         issues_url = URI("#{url}/issues/")
         issues_url.path.squeeze!('/')

spec/controllers/projects/error_tracking_controller_spec.rb

@@ -271,6 +271,58 @@ describe Projects::ErrorTrackingController do
     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
 
   def issue_params(opts = {})

spec/fixtures/api/schemas/error_tracking/update_issue.json

+{
+  "type": "object",
+  "required" : [
+    "result"
+  ],
+  "properties" : {
+    "result": {
+      "type": "object",
+      "properties": {
+        "status": { "type": "string" },
+        "updated": { "type": "boolean" }
+      }
+    }
+  },
+  "additionalProperties": false
+}

spec/lib/sentry/client/issue_spec.rb

@@ -6,7 +6,9 @@ describe Sentry::Client::Issue do
   include SentryClientHelpers
 
   let(:token) { 'test-token' }
+  let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0' }
   let(:client) { Sentry::Client.new(sentry_url, token) }
+  let(:issue_id) { 503504 }
 
   describe '#list_issues' do
     shared_examples 'issues have correct return type' do |klass|
@@ -225,15 +227,11 @@ describe Sentry::Client::Issue do
       )
     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_api_request) { stub_sentry_request(sentry_request_url, body: issue_sample_response) }
 
     subject { client.issue_details(issue_id: issue_id) }
 
-    it_behaves_like 'calls sentry api'
-
     it 'escapes issue ID' do
       allow(CGI).to receive(:escape).and_call_original
 
@@ -290,4 +288,41 @@ describe Sentry::Client::Issue do
       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

spec/models/error_tracking/project_error_tracking_setting_spec.rb

@@ -210,6 +210,40 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
     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
     shared_examples_for 'slug from api_url' do |method, slug|
       context 'when api_url is correct' do

spec/support/helpers/sentry_client_helpers.rb

@@ -3,8 +3,8 @@
 module SentryClientHelpers
   private
 
-  def stub_sentry_request(url, body: {}, status: 200, headers: {})
-    stub_request(:get, url)
+  def stub_sentry_request(url, http_method = :get, body: {}, status: 200, headers: {})
+    stub_request(http_method, url)
       .to_return(
         status: status,
         headers: { 'Content-Type' => 'application/json' }.merge(headers),