Merge branch '14926-limit-amount-of-data-sent-with-sentry-list' into 'master'

Limit amount of data sent with Sentry list

See merge request gitlab-org/gitlab!19649

app/models/error_tracking/project_error_tracking_setting.rb

@@ -7,6 +7,7 @@ module ErrorTracking
 
     SENTRY_API_ERROR_TYPE_MISSING_KEYS = 'missing_keys_in_sentry_response'
     SENTRY_API_ERROR_TYPE_NON_20X_RESPONSE = 'non_20x_response_from_sentry'
+    SENTRY_API_ERROR_INVALID_SIZE = 'invalid_size_of_sentry_response'
 
     API_URL_PATH_REGEXP = %r{
       \A
@@ -116,6 +117,8 @@ module ErrorTracking
       { 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 }
     end
 
     # http://HOST/api/0/projects/ORG/PROJECT

ee/changelogs/unreleased/14926-limit-amount-of-data-sent-with-sentry-list.yml

+---
+title: Enforce a max size accepted for sentry issues list
+merge_request: 19649
+author:
+type: changed

lib/gitlab/utils/deep_size.rb

@@ -25,6 +25,10 @@ module Gitlab
         !too_big? && !too_deep?
       end
 
+      def self.human_default_max_size
+        ActiveSupport::NumberHelper.number_to_human_size(DEFAULT_MAX_SIZE)
+      end
+
       private
 
       def evaluate

lib/sentry/client.rb

@@ -4,6 +4,7 @@ module Sentry
   class Client
     Error = Class.new(StandardError)
     MissingKeysError = Class.new(StandardError)
+    ResponseInvalidSizeError = Class.new(StandardError)
 
     attr_accessor :url, :token
 
@@ -27,6 +28,8 @@ module Sentry
     def list_issues(issue_status:, limit:)
       issues = get_issues(issue_status: issue_status, limit: limit)
 
+      validate_size(issues)
+
       handle_mapping_exceptions do
         map_to_errors(issues)
       end
@@ -42,6 +45,16 @@ module Sentry
 
     private
 
+    def validate_size(issues)
+      return if Gitlab::Utils::DeepSize.new(issues).valid?
+
+      raise Client::ResponseInvalidSizeError, "Sentry API response is too big. Limit is #{Gitlab::Utils::DeepSize.human_default_max_size}."
+    end
+
+    def valid_size?(issues)
+      Gitlab::Utils::DeepSize.new(issues).valid?
+    end
+
     def handle_mapping_exceptions(&block)
       yield
     rescue KeyError => e
@@ -108,7 +121,7 @@ module Sentry
         raise_error "Sentry response status code: #{response.code}"
       end
 
-      response
+      response.parsed_response
     end
 
     def raise_error(message)

spec/lib/gitlab/utils/deep_size_spec.rb

@@ -42,4 +42,10 @@ describe Gitlab::Utils::DeepSize do
       end
     end
   end
+
+  describe '.human_default_max_size' do
+    it 'returns 1 MB' do
+      expect(described_class.human_default_max_size).to eq('1 MB')
+    end
+  end
 end

spec/lib/sentry/client_spec.rb

@@ -192,6 +192,15 @@ describe Sentry::Client do
       end
     end
 
+    context 'sentry api response too large' do
+      it 'raises exception' do
+        deep_size = double('Gitlab::Utils::DeepSize', valid?: false)
+        allow(Gitlab::Utils::DeepSize).to receive(:new).with(sentry_api_response).and_return(deep_size)
+
+        expect { subject }.to raise_error(Sentry::Client::ResponseInvalidSizeError, 'Sentry API response is too big. Limit is 1 MB.')
+      end
+    end
+
     it_behaves_like 'maps exceptions'
   end
 

spec/models/error_tracking/project_error_tracking_setting_spec.rb

@@ -208,6 +208,28 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
         expect(sentry_client).to have_received(:list_issues)
       end
     end
+
+    context 'when sentry client raises Sentry::Client::ResponseInvalidSizeError' do
+      let(:sentry_client) { spy(:sentry_client) }
+      let(:error_msg) {"Sentry API response is too big. Limit is #{Gitlab::Utils::DeepSize.human_default_max_size}."}
+
+      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::ResponseInvalidSizeError, error_msg)
+      end
+
+      it 'returns error' do
+        expect(result).to eq(
+          error: error_msg,
+          error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_INVALID_SIZE
+        )
+        expect(subject).to have_received(:sentry_client)
+        expect(sentry_client).to have_received(:list_issues)
+      end
+    end
   end
 
   describe '#list_sentry_projects' do