From 1c4888fdd018d243b9c920cca9ec5a1cf28798e5 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 23 Oct 2022 21:54:39 +0100 Subject: [PATCH 1/2] Use Exception#detailed_message when generating exception message if applicable Ruby 3.1 provided more detailed exception messages like ``` undefined method `[]' for nil:NilClass {}[:foo][:bar] ^^^^^^ ``` But the additional information was put directly input `Exception#message`, which caused some problems (as described in [this ticket](https://bugs.ruby-lang.org/issues/18438)). So in Ruby 3.2, the additional message will be put into the `Exception#detailed_message`, which as a error monitoring service is what we should use. --- sentry-rails/spec/sentry/rails_spec.rb | 4 +-- .../lib/sentry/interfaces/single_exception.rb | 10 ++++++- sentry-ruby/spec/sentry/client_spec.rb | 30 +++++++++++++++++-- sentry-ruby/spec/sentry/event_spec.rb | 4 +-- .../spec/sentry/sidekiq/error_handler_spec.rb | 2 +- sentry-sidekiq/spec/sentry/sidekiq_spec.rb | 2 +- 6 files changed, 43 insertions(+), 9 deletions(-) diff --git a/sentry-rails/spec/sentry/rails_spec.rb b/sentry-rails/spec/sentry/rails_spec.rb index cfcfff405..7656f4b04 100644 --- a/sentry-rails/spec/sentry/rails_spec.rb +++ b/sentry-rails/spec/sentry/rails_spec.rb @@ -103,7 +103,7 @@ expect(response.status).to eq(500) expect(event["exception"]["values"][0]["type"]).to eq("RuntimeError") - expect(event["exception"]["values"][0]["value"]).to eq("An unhandled exception!") + expect(event["exception"]["values"][0]["value"]).to match("An unhandled exception!") end end @@ -159,7 +159,7 @@ expect(response.status).to eq(500) expect(event["exception"]["values"][0]["type"]).to eq("RuntimeError") - expect(event["exception"]["values"][0]["value"]).to eq("An unhandled exception!") + expect(event["exception"]["values"][0]["value"]).to match("An unhandled exception!") expect(event["sdk"]).to eq("name" => "sentry.ruby.rails", "version" => Sentry::Rails::VERSION) end diff --git a/sentry-ruby/lib/sentry/interfaces/single_exception.rb b/sentry-ruby/lib/sentry/interfaces/single_exception.rb index 6a3ac6e0a..4d08ec5b3 100644 --- a/sentry-ruby/lib/sentry/interfaces/single_exception.rb +++ b/sentry-ruby/lib/sentry/interfaces/single_exception.rb @@ -15,7 +15,15 @@ class SingleExceptionInterface < Interface def initialize(exception:, stacktrace: nil) @type = exception.class.to_s - @value = (exception.message || "").byteslice(0..Event::MAX_MESSAGE_SIZE_IN_BYTES) + exception_message = + if exception.respond_to?(:detailed_message) + exception.detailed_message(highlight: false) + else + exception.message || "" + end + + @value = exception_message.byteslice(0..Event::MAX_MESSAGE_SIZE_IN_BYTES) + @module = exception.class.to_s.split('::')[0...-1].join('::') @thread_id = Thread.current.object_id @stacktrace = stacktrace diff --git a/sentry-ruby/spec/sentry/client_spec.rb b/sentry-ruby/spec/sentry/client_spec.rb index 6283b7a92..491807b0a 100644 --- a/sentry-ruby/spec/sentry/client_spec.rb +++ b/sentry-ruby/spec/sentry/client_spec.rb @@ -161,7 +161,33 @@ def sentry_context it "sets the message to the exception's value and type" do expect(hash[:exception][:values][0][:type]).to eq("Exception") - expect(hash[:exception][:values][0][:value]).to eq(message) + expect(hash[:exception][:values][0][:value]).to match(message) + end + + context "with special error messages" do + let(:exception) do + begin + {}[:foo][:bar] + rescue => e + e + end + end + + it "sets correct exception message based on Ruby version" do + version = Gem::Version.new(RUBY_VERSION) + + if version >= Gem::Version.new("3.2.0-dev") + expect(hash[:exception][:values][0][:value]).to eq( + "undefined method `[]' for nil:NilClass (NoMethodError)\n\n {}[:foo][:bar]\n ^^^^^^" + ) + elsif version >= Gem::Version.new("3.1") + expect(hash[:exception][:values][0][:value]).to eq( + "undefined method `[]' for nil:NilClass\n\n {}[:foo][:bar]\n ^^^^^^" + ) + else + expect(hash[:exception][:values][0][:value]).to eq("undefined method `[]' for nil:NilClass") + end + end end it "sets threads interface without stacktrace" do @@ -195,7 +221,7 @@ def sentry_context it 'returns an event' do event = subject.event_from_exception(ZeroDivisionError.new("divided by 0")) expect(event).to be_a(Sentry::ErrorEvent) - expect(Sentry::Event.get_message_from_exception(event.to_hash)).to eq("ZeroDivisionError: divided by 0") + expect(Sentry::Event.get_message_from_exception(event.to_hash)).to match("ZeroDivisionError: divided by 0") end context 'for a nested exception type' do diff --git a/sentry-ruby/spec/sentry/event_spec.rb b/sentry-ruby/spec/sentry/event_spec.rb index 91c2e6d2f..cfa19b259 100644 --- a/sentry-ruby/spec/sentry/event_spec.rb +++ b/sentry-ruby/spec/sentry/event_spec.rb @@ -224,8 +224,8 @@ end it "returns the exception's message" do - expect(described_class.get_log_message(subject.to_hash)).to eq("Exception: error!") - expect(described_class.get_log_message(subject.to_json_compatible)).to eq("Exception: error!") + expect(described_class.get_log_message(subject.to_hash)).to include("Exception: error!") + expect(described_class.get_log_message(subject.to_json_compatible)).to match("Exception: error!") end end context "with message event" do diff --git a/sentry-sidekiq/spec/sentry/sidekiq/error_handler_spec.rb b/sentry-sidekiq/spec/sentry/sidekiq/error_handler_spec.rb index cf640ac72..cad08d198 100644 --- a/sentry-sidekiq/spec/sentry/sidekiq/error_handler_spec.rb +++ b/sentry-sidekiq/spec/sentry/sidekiq/error_handler_spec.rb @@ -34,7 +34,7 @@ processor.fire_event(:startup) event = transport.events.last.to_hash - expect(Sentry::Event.get_message_from_exception(event)).to eq("RuntimeError: Uhoh!") + expect(Sentry::Event.get_message_from_exception(event)).to match("RuntimeError: Uhoh!") expect(event[:transaction]).to eq "Sidekiq/startup" end diff --git a/sentry-sidekiq/spec/sentry/sidekiq_spec.rb b/sentry-sidekiq/spec/sentry/sidekiq_spec.rb index 677259332..be1bafd5c 100644 --- a/sentry-sidekiq/spec/sentry/sidekiq_spec.rb +++ b/sentry-sidekiq/spec/sentry/sidekiq_spec.rb @@ -46,7 +46,7 @@ event = transport.events.last.to_hash expect(event[:sdk]).to eq({ name: "sentry.ruby.sidekiq", version: described_class::VERSION }) - expect(Sentry::Event.get_message_from_exception(event)).to eq("RuntimeError: I'm sad!") + expect(Sentry::Event.get_message_from_exception(event)).to match("RuntimeError: I'm sad!") end describe "context cleanup" do From 5c96ff641612e83f74ad7d6251df9a68ae5cb337 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 23 Oct 2022 22:29:14 +0100 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ed3fcfc1..507fa9ab3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ - Use `Sentry.with_child_span` in redis and net/http instead of `span.start_child` [#1920](https://github.com/getsentry/sentry-ruby/pull/1920) - This might change the nesting of some spans and make it more accurate +- Use `Exception#detailed_message` when generating exception message if applicable [#1924](https://github.com/getsentry/sentry-ruby/pull/1924) + ### Bug Fixes - `Sentry::BackgroundWorker` will release `ActiveRecord` connection pool only when the `ActiveRecord` connection is established