Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mechanism interface and default to handled false in integrations #2280

Merged
merged 2 commits into from Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,9 @@
## Unreleased

### Features

- Add `Mechanism` interface and default to unhandled for integration exceptions [#2280](https://github.com/getsentry/sentry-ruby/pull/2280)

### Bug Fixes

- Don't instantiate connection in `ActiveRecordSubscriber` ([#2278](https://github.com/getsentry/sentry-ruby/pull/2278))
Expand Down
3 changes: 2 additions & 1 deletion sentry-ruby/lib/sentry/client.rb
Expand Up @@ -88,9 +88,10 @@ def event_from_exception(exception, hint = {})
return if !ignore_exclusions && !@configuration.exception_class_allowed?(exception)

integration_meta = Sentry.integrations[hint[:integration]]
mechanism = hint.delete(:mechanism) { Mechanism.new }

ErrorEvent.new(configuration: configuration, integration_meta: integration_meta).tap do |event|
event.add_exception_interface(exception)
event.add_exception_interface(exception, mechanism: mechanism)
event.add_threads_interface(crashed: true)
event.level = :error
end
Expand Down
4 changes: 2 additions & 2 deletions sentry-ruby/lib/sentry/error_event.rb
Expand Up @@ -27,12 +27,12 @@ def add_threads_interface(backtrace: nil, **options)
end

# @!visibility private
def add_exception_interface(exception)
def add_exception_interface(exception, mechanism:)
if exception.respond_to?(:sentry_context)
@extra.merge!(exception.sentry_context)
end

@exception = Sentry::ExceptionInterface.build(exception: exception, stacktrace_builder: @stacktrace_builder)
@exception = Sentry::ExceptionInterface.build(exception: exception, stacktrace_builder: @stacktrace_builder, mechanism: mechanism)
end
end
end
4 changes: 4 additions & 0 deletions sentry-ruby/lib/sentry/integrable.rb
Expand Up @@ -14,6 +14,10 @@ def integration_name
def capture_exception(exception, **options, &block)
options[:hint] ||= {}
options[:hint][:integration] = integration_name

# within an integration, we usually intercept uncaught exceptions so we set handled to false.
options[:hint][:mechanism] ||= Sentry::Mechanism.new(type: integration_name, handled: false)

Sentry.capture_exception(exception, **options, &block)
end

Expand Down
1 change: 1 addition & 0 deletions sentry-ruby/lib/sentry/interface.rb
Expand Up @@ -14,3 +14,4 @@ def to_hash
require "sentry/interfaces/single_exception"
require "sentry/interfaces/stacktrace"
require "sentry/interfaces/threads"
require "sentry/interfaces/mechanism"
7 changes: 4 additions & 3 deletions sentry-ruby/lib/sentry/interfaces/exception.rb
Expand Up @@ -24,17 +24,18 @@ def to_hash
# @param stacktrace_builder [StacktraceBuilder]
# @see SingleExceptionInterface#build_with_stacktrace
# @see SingleExceptionInterface#initialize
# @param mechanism [Mechanism]
# @return [ExceptionInterface]
def self.build(exception:, stacktrace_builder:)
def self.build(exception:, stacktrace_builder:, mechanism:)
exceptions = Sentry::Utils::ExceptionCauseChain.exception_to_array(exception).reverse
processed_backtrace_ids = Set.new

exceptions = exceptions.map do |e|
if e.backtrace && !processed_backtrace_ids.include?(e.backtrace.object_id)
processed_backtrace_ids << e.backtrace.object_id
SingleExceptionInterface.build_with_stacktrace(exception: e, stacktrace_builder: stacktrace_builder)
SingleExceptionInterface.build_with_stacktrace(exception: e, stacktrace_builder: stacktrace_builder, mechanism: mechanism)
else
SingleExceptionInterface.new(exception: exception)
SingleExceptionInterface.new(exception: exception, mechanism: mechanism)
end
end

Expand Down
20 changes: 20 additions & 0 deletions sentry-ruby/lib/sentry/interfaces/mechanism.rb
@@ -0,0 +1,20 @@
# frozen_string_literal: true

module Sentry
class Mechanism < Interface
# Generic identifier, mostly the source integration for this exception.
# @return [String]
attr_accessor :type

# A manually captured exception has handled set to true,
# false if coming from an integration where we intercept an uncaught exception.
# Defaults to true here and will be set to false explicitly in integrations.
# @return [Boolean]
attr_accessor :handled

def initialize(type: 'generic', handled: true)
@type = type
@handled = handled
end
end
end
10 changes: 6 additions & 4 deletions sentry-ruby/lib/sentry/interfaces/single_exception.rb
Expand Up @@ -11,10 +11,10 @@ class SingleExceptionInterface < Interface
OMISSION_MARK = "...".freeze
MAX_LOCAL_BYTES = 1024

attr_reader :type, :module, :thread_id, :stacktrace
attr_reader :type, :module, :thread_id, :stacktrace, :mechanism
attr_accessor :value

def initialize(exception:, stacktrace: nil)
def initialize(exception:, mechanism:, stacktrace: nil)
@type = exception.class.to_s
exception_message =
if exception.respond_to?(:detailed_message)
Expand All @@ -29,17 +29,19 @@ def initialize(exception:, stacktrace: nil)
@module = exception.class.to_s.split('::')[0...-1].join('::')
@thread_id = Thread.current.object_id
@stacktrace = stacktrace
@mechanism = mechanism
end

def to_hash
data = super
data[:stacktrace] = data[:stacktrace].to_hash if data[:stacktrace]
data[:mechanism] = data[:mechanism].to_hash
data
end

# patch this method if you want to change an exception's stacktrace frames
# also see `StacktraceBuilder.build`.
def self.build_with_stacktrace(exception:, stacktrace_builder:)
def self.build_with_stacktrace(exception:, stacktrace_builder:, mechanism:)
stacktrace = stacktrace_builder.build(backtrace: exception.backtrace)

if locals = exception.instance_variable_get(:@sentry_locals)
Expand All @@ -61,7 +63,7 @@ def self.build_with_stacktrace(exception:, stacktrace_builder:)
stacktrace.frames.last.vars = locals
end

new(exception: exception, stacktrace: stacktrace)
new(exception: exception, stacktrace: stacktrace, mechanism: mechanism)
end
end
end
7 changes: 6 additions & 1 deletion sentry-ruby/lib/sentry/rack/capture_exceptions.rb
Expand Up @@ -4,6 +4,7 @@ module Sentry
module Rack
class CaptureExceptions
ERROR_EVENT_ID_KEY = "sentry.error_event_id"
MECHANISM_TYPE = "rack"

def initialize(app)
@app = app
Expand Down Expand Up @@ -56,7 +57,7 @@ def transaction_op
end

def capture_exception(exception, env)
Sentry.capture_exception(exception).tap do |event|
Sentry.capture_exception(exception, hint: { mechanism: mechanism }).tap do |event|
env[ERROR_EVENT_ID_KEY] = event.event_id if event
end
end
Expand All @@ -74,6 +75,10 @@ def finish_transaction(transaction, status_code)
transaction.set_http_status(status_code)
transaction.finish
end

def mechanism
Sentry::Mechanism.new(type: MECHANISM_TYPE, handled: false)
end
end
end
end
4 changes: 3 additions & 1 deletion sentry-ruby/lib/sentry/rake.rb
Expand Up @@ -8,7 +8,9 @@
module Application
# @api private
def display_error_message(ex)
Sentry.capture_exception(ex) do |scope|
mechanism = Sentry::Mechanism.new(type: 'rake', handled: false)

Check warning on line 11 in sentry-ruby/lib/sentry/rake.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/rake.rb#L11

Added line #L11 was not covered by tests

Sentry.capture_exception(ex, hint: { mechanism: mechanism }) do |scope|
task_name = top_level_tasks.join(' ')
scope.set_transaction_name(task_name, source: :task)
scope.set_tag("rake_task", task_name)
Expand Down
17 changes: 16 additions & 1 deletion sentry-ruby/spec/sentry/client_spec.rb
Expand Up @@ -395,7 +395,7 @@ module ExcTag; end
context "when exclusions overridden with :ignore_exclusions" do
it 'returns Sentry::ErrorEvent' do
config.excluded_exceptions << Sentry::Test::BaseExc
expect(subject.event_from_exception(Sentry::Test::BaseExc.new, ignore_exclusions: true)).to be_a(Sentry::ErrorEvent)
expect(subject.event_from_exception(Sentry::Test::BaseExc.new, { ignore_exclusions: true })).to be_a(Sentry::ErrorEvent)
end
end
end
Expand Down Expand Up @@ -565,6 +565,21 @@ module ExcTag; end
end
end
end

describe 'mechanism' do
it 'has type generic and handled true by default' do
mechanism = hash[:exception][:values][0][:mechanism]
expect(mechanism).to eq({ type: 'generic', handled: true })
end

it 'has correct custom mechanism when passed' do
mech = Sentry::Mechanism.new(type: 'custom', handled: false)
event = subject.event_from_exception(exception, mechanism: mech)
hash = event.to_hash
mechanism = hash[:exception][:values][0][:mechanism]
expect(mechanism).to eq({ type: 'custom', handled: false })
end
end
end

describe "#event_from_check_in" do
Expand Down
8 changes: 7 additions & 1 deletion sentry-ruby/spec/sentry/integrable_spec.rb
Expand Up @@ -48,15 +48,21 @@ module AnotherIntegration; end

it "generates Sentry::FakeIntegration.capture_exception" do
hint = nil
event = nil

Sentry.configuration.before_send = lambda do |event, h|
Sentry.configuration.before_send = lambda do |e, h|
hint = h
event = e
event
end

Sentry::FakeIntegration.capture_exception(exception, hint: { additional_hint: "foo" })

expect(hint).to eq({ additional_hint: "foo", integration: "fake_integration", exception: exception })

mechanism = event.exception.values.first.mechanism
expect(mechanism.type).to eq('fake_integration')
expect(mechanism.handled).to eq(false)
end

it "generates Sentry::FakeIntegration.capture_message" do
Expand Down
11 changes: 11 additions & 0 deletions sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb
Expand Up @@ -25,6 +25,17 @@
expect(last_frame[:vars]).to eq(nil)
end

it 'has the correct mechanism' do
app = ->(_e) { raise exception }
stack = described_class.new(app)

expect { stack.call(env) }.to raise_error(ZeroDivisionError)

event = last_sentry_event.to_hash
mechanism = event.dig(:exception, :values, 0, :mechanism)
expect(mechanism).to eq({ type: 'rack', handled: false })
end

it 'captures the exception from rack.exception' do
app = lambda do |e|
e['rack.exception'] = exception
Expand Down
19 changes: 19 additions & 0 deletions sentry-ruby/spec/sentry_spec.rb
Expand Up @@ -252,6 +252,25 @@
end.to change { sentry_events.count }.by(1)
end

describe 'mechanism' do
it 'has default mechanism' do
event = described_class.capture_exception(exception)
mechanism = event.exception.values.first.mechanism
expect(mechanism).to be_a(Sentry::Mechanism)
expect(mechanism.type).to eq('generic')
expect(mechanism.handled).to eq(true)
end

it 'has custom mechanism if passed' do
mech = Sentry::Mechanism.new(type: 'custom', handled: false)
event = described_class.capture_exception(exception, hint: { mechanism: mech })
mechanism = event.exception.values.first.mechanism
expect(mechanism).to be_a(Sentry::Mechanism)
expect(mechanism.type).to eq('custom')
expect(mechanism.handled).to eq(false)
end
end

context "with include_local_variables = false (default)" do
it "doens't capture local variables" do
begin
Expand Down