Don't report network errors to Sentry#2178
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2178 +/- ##
==========================================
- Coverage 97.34% 97.31% -0.03%
==========================================
Files 99 99
Lines 3690 3691 +1
==========================================
Hits 3592 3592
- Misses 98 99 +1
|
|
@st0012 and this one, pending the changelog resolution ;-) |
| RATE_LIMIT_HEADER = "x-sentry-rate-limits" | ||
| USER_AGENT = "sentry-ruby/#{Sentry::VERSION}" | ||
|
|
||
| NET_HTTP_ERRORS = [ |
There was a problem hiding this comment.
should we maybe use this list from bundler? it has a few more types
There was a problem hiding this comment.
We can!
Pros:
- Better coverage! (I didn't think about Zlib errors)
Cons:
- Assumes Bundler is there. Or wait.
UPD: I think I misunderstood you. I think it's better to just copy that constant over to our code, than require 'bundler' into sentry-ruby?
|
@sl0thentr0py, done — copied the list from Bundler, with two gotchas:
I think in general, having the most common errors is enough. If we miss an error, that's not the end of the world, that's just going to get reported upstream, no big deal. We just need to get 95% right. Right? |
|
yep all good |
|
you'll need to move the changelog entry to a new 'Unreleased' section at the top now that i made a release |
5da83ac to
becc566
Compare
becc566 to
d8a76a8
Compare
|
Done! AFK for 2 hours. |
Summary
This pull request makes it so if Sentry encounters one of the network errors when sending data to Sentry, it won't report that error to Sentry and just re-raise it upstream instead.
Closes #1787.
Changes
::Net::HTTPhas quite a few possible errors, but I opted to rescue potential network errors (alongsideSocketErrorthat was already rescued) instead of rescuingStandardError, so that if something exotic comes up, the user does get a report in Sentry still.Also added a changelog entry.
/cc @sl0thentr0py does that look like an adequate approach to you?