Warn when before_send and before_send_transaction callbacks return non-events

Currently, if the callbacks return non-event and non-nil values, they'd
cause exceptions during serialization, which are rescued and ignored, without
explicit logging.

This commit adds additional checks to the return values so the users
get warnings and also avoids the unnecessary exceptions.

Author: st0012

sentry-ruby/lib/sentry/client.rb

@@ -183,8 +183,11 @@ def send_event(event, hint = nil)
       if event_type != TransactionEvent::TYPE && configuration.before_send
         event = configuration.before_send.call(event, hint)
 
-        if event.nil?
-          log_debug("Discarded event because before_send returned nil")
+        unless event.is_a?(ErrorEvent)
+          # Avoid serializing the event object in this case because we aren't sure what it is and what it contains
+          log_debug(<<~MSG)
+            Discarded event because before_send didn't return a Sentry::ErrorEvent object but an instance of #{event.class}
+          MSG
           transport.record_lost_event(:before_send, data_category)
           return
         end
@@ -193,15 +196,18 @@ def send_event(event, hint = nil)
       if event_type == TransactionEvent::TYPE && configuration.before_send_transaction
         event = configuration.before_send_transaction.call(event, hint)
 
-        if event.nil?
-          log_debug("Discarded event because before_send_transaction returned nil")
-          transport.record_lost_event(:before_send, "transaction")
-          transport.record_lost_event(:before_send, "span", num: spans_before + 1)
-          return
-        else
+        if event.is_a?(TransactionEvent)
           spans_after = event.is_a?(TransactionEvent) ? event.spans.size : 0
           spans_delta = spans_before - spans_after
           transport.record_lost_event(:before_send, "span", num: spans_delta) if spans_delta > 0
+        else
+          # Avoid seraliazing the event object in this case because we aren't sure what it is and what it contains
+          log_debug(<<~MSG)
+            Discarded event because before_send_transaction didn't return a Sentry::TransactionEvent object but an instance of #{event.class}
+          MSG
+          transport.record_lost_event(:before_send, "transaction")
+          transport.record_lost_event(:before_send, "span", num: spans_before + 1)
+          return
         end
       end
 

sentry-ruby/spec/sentry/client/event_sending_spec.rb

@@ -248,6 +248,30 @@
         expect(dbl).not_to receive(:call)
         client.send_event(event)
       end
+
+      it "warns if before_send returns nil" do
+        string_io = StringIO.new
+        logger = Logger.new(string_io, level: :debug)
+        configuration.logger = logger
+        configuration.before_send = lambda do |_event, _hint|
+          nil
+        end
+
+        client.send_event(event)
+        expect(string_io.string).to include("Discarded event because before_send didn't return a Sentry::ErrorEvent object but an instance of NilClass")
+      end
+
+      it "warns if before_send returns non-Event objects" do
+        string_io = StringIO.new
+        logger = Logger.new(string_io, level: :debug)
+        configuration.logger = logger
+        configuration.before_send = lambda do |_event, _hint|
+          123
+        end
+
+        client.send_event(event)
+        expect(string_io.string).to include("Discarded event because before_send didn't return a Sentry::ErrorEvent object but an instance of Integer")
+      end
     end
 
     it_behaves_like "Event in send_event" do
@@ -290,6 +314,18 @@
           expect(event["tags"]["called"]).to eq(true)
         end
       end
+
+      it "warns if before_send_transaction returns nil" do
+        string_io = StringIO.new
+        logger = Logger.new(string_io, level: :debug)
+        configuration.logger = logger
+        configuration.before_send_transaction = lambda do |_event, _hint|
+          nil
+        end
+
+        client.send_event(event)
+        expect(string_io.string).to include("Discarded event because before_send_transaction didn't return a Sentry::TransactionEvent object but an instance of NilClass")
+      end
     end
 
     it_behaves_like "TransactionEvent in send_event" do