diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index b63fe067e..3919b9655 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -10,6 +10,7 @@ require "sentry/utils/argument_checking_helper" require "sentry/utils/encoding_helper" require "sentry/utils/logging_helper" +require "sentry/utils/callback_helper" require "sentry/utils/sample_rand" require "sentry/configuration" require "sentry/structured_logger" diff --git a/sentry-ruby/lib/sentry/backtrace.rb b/sentry-ruby/lib/sentry/backtrace.rb index 020305352..c7e71d661 100644 --- a/sentry-ruby/lib/sentry/backtrace.rb +++ b/sentry-ruby/lib/sentry/backtrace.rb @@ -7,6 +7,8 @@ module Sentry # @api private class Backtrace + extend CallbackHelper + # holder for an Array of Backtrace::Line instances attr_reader :lines @@ -15,7 +17,14 @@ class Backtrace def self.parse(backtrace, project_root, app_dirs_pattern, in_app_pattern: nil, &backtrace_cleanup_callback) ruby_lines = backtrace.is_a?(Array) ? backtrace : backtrace.split(/\n\s*/) - ruby_lines = backtrace_cleanup_callback.call(ruby_lines) if backtrace_cleanup_callback + if backtrace_cleanup_callback + ruby_lines = safe_dispatch_callback( + "backtrace_cleanup_callback", + backtrace_cleanup_callback, + [ruby_lines], + fallback: ruby_lines + ) + end # in_app_pattern is now passed in from StacktraceBuilder, so this regex won't be triggered # only here for backwards compat and will be deleted diff --git a/sentry-ruby/lib/sentry/client.rb b/sentry-ruby/lib/sentry/client.rb index 2bcf497fe..5896bd653 100644 --- a/sentry-ruby/lib/sentry/client.rb +++ b/sentry-ruby/lib/sentry/client.rb @@ -11,6 +11,7 @@ module Sentry class Client include LoggingHelper + include CallbackHelper # The Transport object that'll send events for the client. # @return [Transport] @@ -89,7 +90,7 @@ def capture_event(event, scope, hint = {}) transport.record_lost_event(:queue_overflow, "span", num: spans_before + 1) if is_transaction end else - send_event(event, hint) + event = send_event(event, hint) end event @@ -237,7 +238,11 @@ def send_event(event, hint = nil) spans_before = event.is_a?(TransactionEvent) ? event.spans.size : 0 if event.is_a?(ErrorEvent) && configuration.before_send - event = configuration.before_send.call(event, hint) + event = safe_dispatch_callback( + "before_send", + configuration.before_send, + [event, hint] + ) if !event.is_a?(ErrorEvent) # Avoid serializing the event object in this case because we aren't sure what it is and what it contains @@ -250,7 +255,11 @@ def send_event(event, hint = nil) end if event.is_a?(TransactionEvent) && configuration.before_send_transaction - event = configuration.before_send_transaction.call(event, hint) + event = safe_dispatch_callback( + "before_send_transaction", + configuration.before_send_transaction, + [event, hint] + ) if !event.is_a?(TransactionEvent) # Avoid serializing the event object in this case because we aren't sure what it is and what it contains @@ -268,7 +277,11 @@ def send_event(event, hint = nil) end if event.is_a?(CheckInEvent) && configuration.before_send_check_in - event = configuration.before_send_check_in.call(event, hint) + event = safe_dispatch_callback( + "before_send_check_in", + configuration.before_send_check_in, + [event, hint] + ) if !event.is_a?(CheckInEvent) # Avoid serializing the event object in this case because we aren't sure what it is and what it contains diff --git a/sentry-ruby/lib/sentry/hub.rb b/sentry-ruby/lib/sentry/hub.rb index 81f7aee6f..f8730e517 100644 --- a/sentry-ruby/lib/sentry/hub.rb +++ b/sentry-ruby/lib/sentry/hub.rb @@ -7,6 +7,7 @@ module Sentry class Hub include ArgumentCheckingHelper + include CallbackHelper MUTEX = Mutex.new @@ -296,7 +297,11 @@ def add_breadcrumb(breadcrumb, hint: {}) return unless configuration.enabled_in_current_env? if before_breadcrumb = current_client.configuration.before_breadcrumb - breadcrumb = before_breadcrumb.call(breadcrumb, hint) + breadcrumb = safe_dispatch_callback( + "before_breadcrumb", + before_breadcrumb, + [breadcrumb, hint] + ) end return unless breadcrumb diff --git a/sentry-ruby/lib/sentry/log_event_buffer.rb b/sentry-ruby/lib/sentry/log_event_buffer.rb index 8aceae5e0..d756bc429 100644 --- a/sentry-ruby/lib/sentry/log_event_buffer.rb +++ b/sentry-ruby/lib/sentry/log_event_buffer.rb @@ -21,7 +21,7 @@ def initialize(configuration, client) max_items_before_drop: MAX_EVENTS_BEFORE_DROP, envelope_type: "log", envelope_content_type: "application/vnd.sentry.items.log+json", - before_send: configuration.before_send_log + before_send: :before_send_log ) end end diff --git a/sentry-ruby/lib/sentry/metric_event_buffer.rb b/sentry-ruby/lib/sentry/metric_event_buffer.rb index fe7d8660b..715157697 100644 --- a/sentry-ruby/lib/sentry/metric_event_buffer.rb +++ b/sentry-ruby/lib/sentry/metric_event_buffer.rb @@ -21,7 +21,7 @@ def initialize(configuration, client) max_items_before_drop: MAX_METRICS_BEFORE_DROP, envelope_type: "trace_metric", envelope_content_type: "application/vnd.sentry.items.trace-metric+json", - before_send: configuration.before_send_metric + before_send: :before_send_metric ) end end diff --git a/sentry-ruby/lib/sentry/scope.rb b/sentry-ruby/lib/sentry/scope.rb index 95a8ac0fc..bcf4328ad 100644 --- a/sentry-ruby/lib/sentry/scope.rb +++ b/sentry-ruby/lib/sentry/scope.rb @@ -9,6 +9,7 @@ module Sentry class Scope include ArgumentCheckingHelper + include CallbackHelper ATTRIBUTES = [ :transaction_name, @@ -71,7 +72,8 @@ def apply_to_event(event, hint = nil) unless all_event_processors.empty? all_event_processors.each do |processor_block| - event = processor_block.call(event, hint) + event = safe_dispatch_callback("event_processor", processor_block, [event, hint]) + return unless event end end diff --git a/sentry-ruby/lib/sentry/std_lib_logger.rb b/sentry-ruby/lib/sentry/std_lib_logger.rb index 83bdd9a5a..fff5d590c 100644 --- a/sentry-ruby/lib/sentry/std_lib_logger.rb +++ b/sentry-ruby/lib/sentry/std_lib_logger.rb @@ -4,6 +4,8 @@ module Sentry # Ruby Logger support Add commentMore actions # intercepts any logger instance and send the log to Sentry too. module StdLibLogger + include CallbackHelper + SEVERITY_MAP = { 0 => :debug, 1 => :info, @@ -37,8 +39,12 @@ def add(severity, message = nil, progname = nil, &block) message = message.to_s.strip if !message.nil? && message != Sentry::Logger::PROGNAME && method = SEVERITY_MAP[severity] - if (filter = Sentry.configuration.std_lib_logger_filter) && !filter.call(self, message, method) - return result + if filter = Sentry.configuration.std_lib_logger_filter + return result unless safe_dispatch_callback( + "std_lib_logger_filter", + filter, + [self, message, method] + ) end Sentry.logger.send(method, message, origin: ORIGIN) diff --git a/sentry-ruby/lib/sentry/telemetry_event_buffer.rb b/sentry-ruby/lib/sentry/telemetry_event_buffer.rb index 714957943..790702d0e 100644 --- a/sentry-ruby/lib/sentry/telemetry_event_buffer.rb +++ b/sentry-ruby/lib/sentry/telemetry_event_buffer.rb @@ -12,6 +12,8 @@ module Sentry # # @!visibility private class TelemetryEventBuffer < ThreadedPeriodicWorker + include CallbackHelper + FLUSH_INTERVAL = 5 # seconds # @!visibility private @@ -21,6 +23,7 @@ def initialize(configuration, client, event_class:, max_items:, max_items_before super(configuration.sdk_logger, FLUSH_INTERVAL) @client = client + @configuration = configuration @dsn = configuration.dsn @debug = configuration.debug @event_class = event_class @@ -95,9 +98,9 @@ def send_items discarded_bytes = 0 envelope_items = [] - if @before_send + if callback = @configuration.send(@before_send) @pending_items.each do |item| - processed_item = @before_send.call(item) + processed_item = safe_dispatch_callback(@before_send.to_s, callback, [item]) if processed_item envelope_items << processed_item.to_h diff --git a/sentry-ruby/lib/sentry/transaction.rb b/sentry-ruby/lib/sentry/transaction.rb index 01d23f345..33107340e 100644 --- a/sentry-ruby/lib/sentry/transaction.rb +++ b/sentry-ruby/lib/sentry/transaction.rb @@ -14,6 +14,7 @@ class Transaction < Span SOURCES = %i[custom url route view component task] include LoggingHelper + include CallbackHelper # The name of the transaction. # @return [String] @@ -142,7 +143,12 @@ def set_initial_sample_decision(sampling_context:) sample_rate = if configuration.traces_sampler.is_a?(Proc) - configuration.traces_sampler.call(sampling_context) + safe_dispatch_callback( + "traces_sampler", + configuration.traces_sampler, + [sampling_context], + fallback: configuration.traces_sample_rate + ) elsif !sampling_context[:parent_sampled].nil? sampling_context[:parent_sampled] else diff --git a/sentry-ruby/lib/sentry/utils/callback_helper.rb b/sentry-ruby/lib/sentry/utils/callback_helper.rb new file mode 100644 index 000000000..91da0b4cf --- /dev/null +++ b/sentry-ruby/lib/sentry/utils/callback_helper.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Sentry + # @private + module CallbackHelper + # @!visibility private + def safe_dispatch_callback(callback_name, callback, args, fallback: nil) + callback.call(*args) + rescue => e + log_callback_error(callback_name, e) + fallback + end + + private + + def log_callback_error(callback_name, exception) + return unless Sentry.initialized? + + message = "Error in #{callback_name} callback: #{exception.message}" + message += "\n#{exception.backtrace.join("\n")}" if Sentry.configuration.debug && exception.backtrace + + Sentry.sdk_logger.error(LOGGER_PROGNAME) { message } + rescue StandardError + # Callback error reporting must not propagate to the application. + end + end +end diff --git a/sentry-ruby/spec/sentry/client/event_sending_spec.rb b/sentry-ruby/spec/sentry/client/event_sending_spec.rb index e60fc1337..0322be968 100644 --- a/sentry-ruby/spec/sentry/client/event_sending_spec.rb +++ b/sentry-ruby/spec/sentry/client/event_sending_spec.rb @@ -12,6 +12,11 @@ before do stub_request(:post, Sentry::TestHelper::DUMMY_DSN) allow(Sentry).to receive(:configuration).and_return configuration + allow(Sentry).to receive(:initialized?).and_return(true) + end + + after do + allow(Sentry).to receive(:initialized?).and_call_original end subject(:client) { Sentry::Client.new(configuration) } @@ -373,7 +378,7 @@ it "swallows the event and logs the failure" do expect(client.capture_event(event, scope)).to be_nil - expect(string_io.string).to match(/Event capturing failed: TypeError/) + expect(string_io.string).to match(/Error in event_processor callback: TypeError/) expect(string_io.string).not_to match(__FILE__) end @@ -384,7 +389,7 @@ it "logs the error with backtrace" do expect(client.capture_event(event, scope)).to be_nil - expect(string_io.string).to match(/Event capturing failed: TypeError/) + expect(string_io.string).to match(/Error in event_processor callback: TypeError/) expect(string_io.string).to match(__FILE__) end end @@ -411,7 +416,9 @@ expect(client.capture_event(event, scope)).to be_nil - expect(string_io.string).to match(/Event sending failed: TypeError/) + expect(string_io.string).to match(/Error in before_send callback: TypeError/) + expect(client.transport).to have_recorded_lost_event(:before_send, "error") + expect(client.transport).not_to have_recorded_lost_event(:network_error, "error") end it "captures client report for error event" do @@ -447,7 +454,9 @@ expect(client.capture_event(event, scope)).to be_a(Sentry::ErrorEvent) sleep(0.2) - expect(string_io.string).to match(/Event sending failed: TypeError/) + expect(string_io.string).to match(/Error in before_send callback: TypeError/) + expect(client.transport).to have_recorded_lost_event(:before_send, "error") + expect(client.transport).not_to have_recorded_lost_event(:network_error, "error") end it "captures client report for error event" do @@ -486,12 +495,12 @@ end end - it "raises the error" do - expect do - client.send_event(event) - end.to raise_error(TypeError) + it "swallows and logs the error" do + expect(client.send_event(event)).to be_nil - expect(string_io.string).to match(/Event sending failed: TypeError/) + expect(string_io.string).to match(/Error in before_send callback: TypeError/) + expect(client.transport).to have_recorded_lost_event(:before_send, "error") + expect(client.transport).not_to have_recorded_lost_event(:network_error, "error") end context "with config.debug = true" do @@ -500,11 +509,9 @@ end it "logs the error with backtrace" do - expect do - client.send_event(event) - end.to raise_error(TypeError) + expect(client.send_event(event)).to be_nil - expect(string_io.string).to match(/Event sending failed: TypeError/) + expect(string_io.string).to match(/Error in before_send callback: TypeError/) expect(string_io.string).to match(__FILE__) end end diff --git a/sentry-ruby/spec/sentry/hub_spec.rb b/sentry-ruby/spec/sentry/hub_spec.rb index 8885e2fef..1475d2bc5 100644 --- a/sentry-ruby/spec/sentry/hub_spec.rb +++ b/sentry-ruby/spec/sentry/hub_spec.rb @@ -481,6 +481,25 @@ expect(peek_crumb).to eq(nil) end end + + context "when before_breadcrumb raises" do + before do + allow(Sentry).to receive(:configuration).and_return(configuration) + allow(Sentry).to receive(:initialized?).and_return(true) + configuration.before_breadcrumb = ->(_breadcrumb, _hint) { raise TypeError } + end + + after do + allow(Sentry).to receive(:initialized?).and_call_original + end + + it "logs the error and doesn't add anything" do + expect { subject.add_breadcrumb(new_breadcrumb) }.not_to raise_error + + expect(peek_crumb).to eq(nil) + expect(string_io.string).to include("Error in before_breadcrumb callback: TypeError") + end + end end context "when the SDK is not activated in the current environment" do @@ -609,7 +628,7 @@ let(:message) { "Test message" } it 'sends the result of Event.capture_type' do - expect(client).to receive(:send_event) + expect(client).to receive(:send_event).and_call_original event = subject.capture_message("Test message") diff --git a/sentry-ruby/spec/sentry/metrics_spec.rb b/sentry-ruby/spec/sentry/metrics_spec.rb index 970568998..dfbabdc64 100644 --- a/sentry-ruby/spec/sentry/metrics_spec.rb +++ b/sentry-ruby/spec/sentry/metrics_spec.rb @@ -316,6 +316,8 @@ end context "with before_send_metric callback" do + let(:string_io) { StringIO.new } + it "receives MetricEvent" do perform_basic_setup do |config| config.before_send_metric = lambda do |metric| @@ -361,6 +363,20 @@ expect(sentry_metrics.first[:name]).to eq("test.allowed") expect(Sentry.get_current_client.transport).to have_recorded_lost_event(:before_send, 'trace_metric', num: 2, num_bytes: a_value > 0) end + + it "drops metrics when the callback raises" do + perform_basic_setup do |config| + config.sdk_logger = Logger.new(string_io) + config.before_send_metric = ->(_metric) { raise TypeError } + end + + expect { Sentry.metrics.count("test.failed") }.not_to raise_error + Sentry.get_current_client.flush + + expect(sentry_metrics).to be_empty + expect(Sentry.get_current_client.transport).to have_recorded_lost_event(:before_send, 'trace_metric', num: 1, num_bytes: a_value > 0) + expect(string_io.string).to include("Error in before_send_metric callback: TypeError") + end end end end diff --git a/sentry-ruby/spec/sentry/structured_logger_spec.rb b/sentry-ruby/spec/sentry/structured_logger_spec.rb index 94a68801a..256445c91 100644 --- a/sentry-ruby/spec/sentry/structured_logger_spec.rb +++ b/sentry-ruby/spec/sentry/structured_logger_spec.rb @@ -258,6 +258,24 @@ expect(transport.discarded_events[[:before_send, "log_byte"]]).to be > 0 end end + + context "when the callback raises" do + let(:send_client_reports) { true } + let(:before_send_log) { ->(_log) { raise TypeError } } + let(:string_io) { StringIO.new } + + before do + Sentry.configuration.sdk_logger = Logger.new(string_io) + end + + it "drops the log and records a discarded event" do + expect { Sentry.logger.info("Hello World") }.not_to raise_error + + expect(sentry_logs).to be_empty + expect(transport.discarded_events).to include([:before_send, "log_item"] => 1) + expect(string_io.string).to include("Error in before_send_log callback: TypeError") + end + end end end end diff --git a/sentry-ruby/spec/sentry/transaction_spec.rb b/sentry-ruby/spec/sentry/transaction_spec.rb index 4b2b61572..0f8fe05b9 100644 --- a/sentry-ruby/spec/sentry/transaction_spec.rb +++ b/sentry-ruby/spec/sentry/transaction_spec.rb @@ -209,6 +209,19 @@ expect(subject.effective_sample_rate).to eq(0.0) end + it "falls back to traces_sample_rate when traces_sampler raises" do + Sentry.configuration.traces_sample_rate = 1.0 + Sentry.configuration.traces_sampler = ->(_) { raise TypeError } + + expect do + subject.set_initial_sample_decision(sampling_context: {}) + end.not_to raise_error + + expect(subject.sampled).to eq(true) + expect(subject.effective_sample_rate).to eq(1.0) + expect(string_io.string).to include("Error in traces_sampler callback: TypeError") + end + it "prioritizes traces_sampler over inherited decision" do Sentry.configuration.traces_sampler = ->(_) { false }