Merge branch 'bvl-report-recoverable-errors' into 'master'

config/initializers/forbid_sidekiq_in_transactions.rb

@@ -27,16 +27,8 @@ module Sidekiq
                 Use an `after_commit` hook, or include `AfterCommitQueue` and use a `run_after_commit` block instead.
                 MSG
               rescue Sidekiq::Worker::EnqueueFromTransactionError => e
-                if Rails.env.production?
-                  Rails.logger.error(e.message)
-
-                  if Gitlab::Sentry.enabled?
-                    Gitlab::Sentry.context
-                    Raven.capture_exception(e)
-                  end
-                else
-                  raise
-                end
+                Rails.logger.error(e.message) if Rails.env.production?
+                Gitlab::Sentry.track_exception(e)
               end
             end
 

lib/gitlab/diff/highlight.rb

@@ -33,10 +33,7 @@ module Gitlab
             # match the blob, which is a bug. But we shouldn't fail to render
             # completely in that case, even though we want to report the error.
             rescue RangeError => e
-              if Gitlab::Sentry.enabled?
-                Gitlab::Sentry.context
-                Raven.capture_exception(e)
-              end
+              Gitlab::Sentry.track_exception(e, issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/45441')
             end
           end
 

lib/gitlab/sentry.rb

@@ -18,6 +18,25 @@ module Gitlab
       end
     end
 
+    # This can be used for investigating exceptions that can be recovered from in
+    # code. The exception will still be raised in development and test
+    # environments.
+    #
+    # That way we can track down these exceptions with as much information as we
+    # need to resolve them.
+    #
+    # Provide an issue URL for follow up.
+    def self.track_exception(exception, issue_url: nil, extra: {})
+      if enabled?
+        extra[:issue_url] = issue_url if issue_url
+        context # Make sure we've set everything we know in the context
+
+        Raven.capture_exception(exception, extra: extra)
+      end
+
+      raise exception if should_raise?
+    end
+
     def self.program_context
       if Sidekiq.server?
         'sidekiq'
@@ -25,5 +44,9 @@ module Gitlab
         'rails'
       end
     end
+
+    def self.should_raise?
+      Rails.env.development? || Rails.env.test?
+    end
   end
 end

spec/lib/gitlab/diff/highlight_spec.rb

@@ -79,6 +79,8 @@ describe Gitlab::Diff::Highlight do
         end
 
         it 'keeps the original rich line' do
+          allow(Gitlab::Sentry).to receive(:track_exception)
+
           code = %q{+      raise RuntimeError, "System commands must be given as an array of strings"}
 
           expect(subject[5].text).to eq(code)
@@ -86,12 +88,9 @@ describe Gitlab::Diff::Highlight do
         end
 
         it 'reports to Sentry if configured' do
-          allow(Gitlab::Sentry).to receive(:enabled?).and_return(true)
-
-          expect(Gitlab::Sentry).to receive(:context)
-          expect(Raven).to receive(:capture_exception)
+          expect(Gitlab::Sentry).to receive(:track_exception).and_call_original
 
-          subject
+          expect { subject }. to raise_exception(RangeError)
         end
       end
     end

spec/lib/gitlab/sentry_spec.rb

@@ -7,7 +7,49 @@ describe Gitlab::Sentry do
 
       described_class.context(nil)
 
-      expect(Raven.tags_context[:locale]).to eq(I18n.locale.to_s)
+      expect(Raven.tags_context[:locale].to_s).to eq(I18n.locale.to_s)
+    end
+  end
+
+  describe '.track_exception' do
+    let(:exception) { RuntimeError.new('boom') }
+
+    before do
+      allow(described_class).to receive(:enabled?).and_return(true)
+    end
+
+    it 'raises the exception if it should' do
+      expect(described_class).to receive(:should_raise?).and_return(true)
+      expect { described_class.track_exception(exception) }
+        .to raise_error(RuntimeError)
+    end
+
+    context 'when exceptions should not be raised' do
+      before do
+        allow(described_class).to receive(:should_raise?).and_return(false)
+      end
+
+      it 'logs the exception with all attributes passed' do
+        expected_extras = {
+          some_other_info: 'info',
+          issue_url: 'http://gitlab.com/gitlab-org/gitlab-ce/issues/1'
+        }
+
+        expect(Raven).to receive(:capture_exception)
+                           .with(exception, extra: a_hash_including(expected_extras))
+
+        described_class.track_exception(
+          exception,
+          issue_url: 'http://gitlab.com/gitlab-org/gitlab-ce/issues/1',
+          extra: { some_other_info: 'info' }
+        )
+      end
+
+      it 'sets the context' do
+        expect(described_class).to receive(:context)
+
+        described_class.track_exception(exception)
+      end
     end
   end
 end