Skip to content

Commit

Permalink
Use Exception#detailed_message when generating exception message if…
Browse files Browse the repository at this point in the history
… applicable (#1924)

* 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.

* Update changelog
  • Loading branch information
st0012 authored Oct 25, 2022
1 parent a1fda66 commit 7d97914
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions sentry-rails/spec/sentry/rails_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
10 changes: 9 additions & 1 deletion sentry-ruby/lib/sentry/interfaces/single_exception.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 28 additions & 2 deletions sentry-ruby/spec/sentry/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions sentry-ruby/spec/sentry/event_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion sentry-sidekiq/spec/sentry/sidekiq/error_handler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion sentry-sidekiq/spec/sentry/sidekiq_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 7d97914

Please sign in to comment.