fix: Do not capture handled exceptions in sanic #127
Conversation
Ignore exceptions with error handlers defined by the sanic application.
Codecov Report
@@ Coverage Diff @@
## master #127 +/- ##
==========================================
+ Coverage 66.98% 74.71% +7.72%
==========================================
Files 25 25
Lines 2484 2167 -317
Branches 353 376 +23
==========================================
- Hits 1664 1619 -45
+ Misses 687 420 -267
+ Partials 133 128 -5
Continue to review full report at Codecov.
|
|
We intentionally did it this way across all framework integrations because an errorhandler for a 500 Internal Server error would suppress all errors. I would recommend configuring the SDK separately for ignored errors (using |
|
@untitaker I can understand your decision to handle it this way. But, it does feel a bit counter intuitive to consider an error to be not handled, when it has explicitly been done so in the application. In the case of sanic, when an error handler is defined for def test_error_in_errorhandler_500(sentry_init, app, capture_events):
sentry_init(integrations=[SanicIntegration()])
events = capture_events()
@app.route("/error")
def myerror(request):
raise ValueError("oh no")
@app.exception(SanicException)
def myhandler(request, exception):
return response.json({}, 500)
request, response = app.test_client.get("/error")
assert response.status == 500
assert len(events) == 1
exception, = events[0]["exception"]["values"]
assert exception["type"] == "ValueError"
assert any(
frame["filename"].endswith("test_sanic.py")
for frame in exception["stacktrace"]["frames"]
)Could this behavior be something that can be configured using a flag when initializing the integration? Would appreciate your thoughts on this. In any case, we might mute for now and use the |
|
If I want to have a custom error response for all kinds of 500 Internal Server Errors, I would register a route |
|
@abn I believe I have a good solution for your problem. I suppose what you want is to ignore SanicException in the integration |
|
@untitaker for our current issue I think your solution in b50b0b3 would do the trick. Thanks for that one. |
Ignore exceptions with error handlers defined by the sanic application.
Proposing this change in behavior when handling exceptions thrown by a Sanic application as the current implementation triggers issue events for "known" and gracefully handled exceptions and does not allow these to be suppressed at the client side. An example of this, is a case where a client request to a websocket route causes an
InvalidHandshakeexception which in turn, is handled by sanic to raise anInvalidUsageexception. These exceptions, if a handler is registered explicitly in the application, should not create an issue event as this can lead to a large number of events generated for a heavily used websocket endpoint. Additionally, in any such scenarios an event can be captured by either explicitly using acapture_messageor initializing the logging integration and logging an error.The proposed change retains capturing any errors that may be encountered during the error handler execution.