Skip to content

Support config.debug configuration option#1400

Merged
st0012 merged 4 commits intomasterfrom
implement-#1297
Apr 23, 2021
Merged

Support config.debug configuration option#1400
st0012 merged 4 commits intomasterfrom
implement-#1297

Conversation

@st0012
Copy link
Copy Markdown
Contributor

@st0012 st0012 commented Apr 16, 2021

This PR adds the new debug config option:

config.debug = true # default is false

As of now, SDK related errors are only logged with their error messages.

For example, assume we have this before_send callback:

  config.before_send = lambda do |event, hint|
    foo
  end

The SDK only logs this

Event sending failed: undefined local variable or method `foo' for main:Object

This is inconvenient for our users to debug their customization on the SDK. It also makes determining/reporting SDK issues harder (see #1383 (comment) for example).

So with when the new config.debug option set with true, it will log SDK errors with backtrace, just like we normally treat errors in our applications.

Event sending failed: undefined local variable or method `foo' for main:Object
/Users/st0012/projects/sentry-ruby/sentry-rails/examples/rails-6.0/config/initializers/sentry.rb:10:in `block (2 levels) in <main>'
/Users/st0012/projects/sentry-ruby/sentry-ruby/lib/sentry/client.rb:78:in `send_event'
/Users/st0012/projects/sentry-ruby/sentry-ruby/lib/sentry/client.rb:102:in `block in dispatch_background_event'

Closes #1297

@st0012 st0012 added this to the 4.4.0 milestone Apr 16, 2021
@st0012 st0012 self-assigned this Apr 16, 2021
@st0012 st0012 changed the title Support config.debug option Support config.debug configuration option Apr 16, 2021
@st0012 st0012 requested a review from rhcarvalho April 19, 2021 06:49
@st0012 st0012 force-pushed the implement-#1297 branch 2 times, most recently from 8fc97f1 to 4ba5b41 Compare April 22, 2021 13:51
Copy link
Copy Markdown
Contributor

@rhcarvalho rhcarvalho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm misreading this, but it seems that regardless of debug = true/false we're always printing full event payloads to STDOUT?

@st0012 st0012 requested a review from rhcarvalho April 22, 2021 15:29
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 22, 2021

Codecov Report

Merging #1400 (2e0ee8c) into master (615db8c) will increase coverage by 0.62%.
The diff coverage is 100.00%.

❗ Current head 2e0ee8c differs from pull request most recent head 6cd5e33. Consider uploading reports for the commit 6cd5e33 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1400      +/-   ##
==========================================
+ Coverage   98.20%   98.82%   +0.62%     
==========================================
  Files         213      118      -95     
  Lines        9876     6068    -3808     
==========================================
- Hits         9699     5997    -3702     
+ Misses        177       71     -106     
Impacted Files Coverage Δ
sentry-ruby/lib/sentry/client.rb 97.18% <100.00%> (+0.04%) ⬆️
sentry-ruby/lib/sentry/configuration.rb 97.93% <100.00%> (+0.02%) ⬆️
sentry-ruby/lib/sentry/utils/logging_helper.rb 100.00% <100.00%> (ø)
...ntry-ruby/spec/sentry/client/event_sending_spec.rb 99.50% <100.00%> (+0.04%) ⬆️
...try-raven/lib/sentry-raven-without-integrations.rb
sentry-raven/lib/raven/interfaces/exception.rb
sentry-raven/lib/raven/integrations/rack.rb
...en/lib/raven/integrations/sidekiq/error_handler.rb
...integrations/rails/overrides/streaming_reporter.rb
...ven/spec/raven/utils/exception_cause_chain_spec.rb
... and 89 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 615db8c...6cd5e33. Read the comment docs.

Copy link
Copy Markdown
Contributor

@rhcarvalho rhcarvalho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @st0012 !

@st0012 st0012 merged commit 8f740de into master Apr 23, 2021
@st0012 st0012 deleted the implement-#1297 branch April 23, 2021 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support debug option

3 participants