Improve SDK's error handling

sentry-ruby/CHANGELOG.md

@@ -3,6 +3,9 @@
 ## Unreleased
 
 - Refactor interface construction [#1296](https://github.com/getsentry/sentry-ruby/pull/1296)
+- Improve SDK's error handling [#1298](https://github.com/getsentry/sentry-ruby/pull/1298)
+  - Fixes [#1246](https://github.com/getsentry/sentry-ruby/issues/1246) and [#1289](https://github.com/getsentry/sentry-ruby/issues/1289)
+  - Please read [#1290](https://github.com/getsentry/sentry-ruby/issues/1290) to see the full specification
 - Treat query string as pii too [#1302](https://github.com/getsentry/sentry-ruby/pull/1302)
   - Fixes [#1301](https://github.com/getsentry/sentry-ruby/issues/1301)
 - Ignore sentry-trace when tracing is not enabled [#1308](https://github.com/getsentry/sentry-ruby/pull/1308)

sentry-ruby/lib/sentry-ruby.rb

@@ -3,6 +3,7 @@
 require "time"
 
 require "sentry/version"
+require "sentry/exceptions"
 require "sentry/core_ext/object/deep_dup"
 require "sentry/utils/argument_checking_helper"
 require "sentry/configuration"
@@ -26,9 +27,6 @@
 end
 
 module Sentry
-  class Error < StandardError
-  end
-
   META = { "name" => "sentry.ruby", "version" => Sentry::VERSION }.freeze
 
   LOGGER_PROGNAME = "sentry".freeze

sentry-ruby/lib/sentry/background_worker.rb

@@ -1,5 +1,6 @@
 require "concurrent/executor/thread_pool_executor"
 require "concurrent/executor/immediate_executor"
+require "concurrent/configuration"
 
 module Sentry
   class BackgroundWorker

sentry-ruby/lib/sentry/client.rb

@@ -2,10 +2,11 @@
 
 module Sentry
   class Client
-    attr_reader :transport, :configuration
+    attr_reader :transport, :configuration, :logger
 
     def initialize(configuration)
       @configuration = configuration
+      @logger = configuration.logger
 
       if transport_class = configuration.transport.transport_class
         @transport = transport_class.new(configuration)
@@ -26,32 +27,17 @@ def capture_event(event, scope, hint = {})
       scope.apply_to_event(event, hint)
 
       if async_block = configuration.async
-        begin
-          # We have to convert to a JSON-like hash, because background job
-          # processors (esp ActiveJob) may not like weird types in the event hash
-          event_hash = event.to_json_compatible
-
-          if async_block.arity == 2
-            hint = JSON.parse(JSON.generate(hint))
-            async_block.call(event_hash, hint)
-          else
-            async_block.call(event_hash)
-          end
-        rescue => e
-          configuration.logger.error(LOGGER_PROGNAME) { "async event sending failed: #{e.message}" }
-          send_event(event, hint)
-        end
+        dispatch_async_event(async_block, event, hint)
+      elsif hint.fetch(:background, true)
+        dispatch_background_event(event, hint)
       else
-        if hint.fetch(:background, true)
-          Sentry.background_worker.perform do
-            send_event(event, hint)
-          end
-        else
-          send_event(event, hint)
-        end
+        send_event(event, hint)
       end
 
       event
+    rescue => e
+      logger.error(LOGGER_PROGNAME) { "Event capturing failed: #{e.message}" }
+      nil
     end
 
     def event_from_exception(exception, hint = {})
@@ -85,16 +71,49 @@ def event_from_transaction(transaction)
 
     def send_event(event, hint = nil)
       event_type = event.is_a?(Event) ? event.type : event["type"]
-      event = configuration.before_send.call(event, hint) if configuration.before_send && event_type == "event"
 
-      if event.nil?
-        configuration.logger.info(LOGGER_PROGNAME) { "Discarded event because before_send returned nil" }
-        return
+      if event_type == "event" && configuration.before_send
+        event = configuration.before_send.call(event, hint)
+
+        if event.nil?
+          logger.info(LOGGER_PROGNAME) { "Discarded event because before_send returned nil" }
+          return
+        end
       end
 
       transport.send_event(event)
 
       event
+    rescue => e
+      logger.error(LOGGER_PROGNAME) { "#{event_type.capitalize} sending failed: #{e.message}" }
+      logger.error(LOGGER_PROGNAME) { "Unreported #{event_type.capitalize}: #{Event.get_log_message(event.to_hash)}" }
+      raise
     end
+
+    private
+
+    def dispatch_background_event(event, hint)
+      Sentry.background_worker.perform do
+        send_event(event, hint)
+      end
+    end
+
+    def dispatch_async_event(async_block, event, hint)
+      # We have to convert to a JSON-like hash, because background job
+      # processors (esp ActiveJob) may not like weird types in the event hash
+      event_hash = event.to_json_compatible
+
+      if async_block.arity == 2
+        hint = JSON.parse(JSON.generate(hint))
+        async_block.call(event_hash, hint)
+      else
+        async_block.call(event_hash)
+      end
+    rescue => e
+      event_type = event_hash["type"]
+      logger.error(LOGGER_PROGNAME) { "Async #{event_type} sending failed: #{e.message}" }
+      send_event(event, hint)
+    end
+
   end
 end

sentry-ruby/lib/sentry/event.rb

@@ -52,19 +52,26 @@ def initialize(configuration:, integration_meta: nil, message: nil)
     class << self
       def get_log_message(event_hash)
         message = event_hash[:message] || event_hash['message']
-        message = get_message_from_exception(event_hash) if message.nil? || message.empty?
-        message = '<no message value>' if message.nil? || message.empty?
-        message
+
+        return message unless message.nil? || message.empty?
+
+        message = get_message_from_exception(event_hash)
+
+        return message unless message.nil? || message.empty?
+
+        message = event_hash[:transaction] || event_hash["transaction"]
+
+        return message unless message.nil? || message.empty?
+
+        '<no message value>'
       end
 
       def get_message_from_exception(event_hash)
-        (
-          event_hash &&
-          event_hash[:exception] &&
-          event_hash[:exception][:values] &&
-          event_hash[:exception][:values][0] &&
-          "#{event_hash[:exception][:values][0][:type]}: #{event_hash[:exception][:values][0][:value]}"
-        )
+        if exception = event_hash.dig(:exception, :values, 0)
+          "#{exception[:type]}: #{exception[:value]}"
+        elsif exception = event_hash.dig("exception", "values", 0)
+          "#{exception["type"]}: #{exception["value"]}"
+        end
       end
     end
 

sentry-ruby/lib/sentry/exceptions.rb

@@ -0,0 +1,7 @@
+module Sentry
+  class Error < StandardError
+  end
+
+  class ExternalError < StandardError
+  end
+end

sentry-ruby/lib/sentry/transport.rb

@@ -31,9 +31,6 @@ def send_event(event)
       send_data(encoded_data)
 
       event
-    rescue => e
-      failed_for_exception(e, event)
-      nil
     end
 
     def generate_auth_header
@@ -72,15 +69,6 @@ def prepare_encoded_event(event)
       configuration.logger.info(LOGGER_PROGNAME) { "Sending #{event_type} #{event_id} to Sentry" }
       encode(event_hash)
     end
-
-    def failed_for_exception(e, event)
-      configuration.logger.warn(LOGGER_PROGNAME) { "Unable to record event with remote Sentry server (#{e.class} - #{e.message}):\n#{e.backtrace[0..10].join("\n")}" }
-      log_not_sending(event)
-    end
-
-    def log_not_sending(event)
-      configuration.logger.warn(LOGGER_PROGNAME) { "Failed to submit event. Unreported Event: #{Event.get_log_message(event.to_hash)}" }
-    end
   end
 end
 

sentry-ruby/lib/sentry/transport/http_transport.rb

@@ -38,7 +38,7 @@ def send_data(data)
         error_info += " Error in headers is: #{e.response[:headers]['x-sentry-error']}" if e.response[:headers]['x-sentry-error']
       end
 
-      raise Sentry::Error, error_info
+      raise Sentry::ExternalError, error_info
     end
 
     private