Improve before_send and before_send_transaction's return value handling

CHANGELOG.md

@@ -7,6 +7,7 @@
 ### Bug fixes
 
 - Default to `internal_error` error type for OpenTelemetry spans [#2473](https://github.com/getsentry/sentry-ruby/pull/2473)
+- Improve `before_send` and `before_send_transaction`'s return value handling ([#2504](https://github.com/getsentry/sentry-ruby/pull/2504))
 
 ## 5.22.1
 

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 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_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

sentry-ruby/spec/spec_helper.rb

@@ -26,7 +26,8 @@
 require "sentry-ruby"
 require "sentry/test_helper"
 
-Dir[Pathname(__dir__).join("support/**/*.rb")].sort.each { |f| require f }
+require "support/profiler"
+require "support/stacktrace_test_fixture"
 
 RSpec.configure do |config|
   # Enable flags like --only-failures and --next-failure