getsentry / sentry-python

The official Python SDK for Sentry.io
https://sentry.io/for/python/
MIT License
1.92k stars 507 forks source link

Sanic Integration: Handled exceptions get captured and sent to sentry #794

Closed simonlindroosfyndiq closed 2 years ago

simonlindroosfyndiq commented 4 years ago

Hi,

I have an issue that the Sanic integration is capturing and sending exceptions to Sentry even though I have declared an exception handler using the @app.exception decorator as described here Sanic Exceptions. I have created a sample application that reproduces this issue. The Sentry DSN key is provided in the SENTRY_DSN environment variable. I have tried this using python 3.7.5, 3.8.0 and 3.8.5 as well as with older versions of the sentry_sdk library without success. I have found that the exception is being sent to sentry before the code reaches the error handler, noted by the comment in the code below.

Note that if I remove the SanicIntegration() during initialisation of the sentry_sdk, the application behaves as expected. Only unhandled exceptions are sent to Sentry but with less context about the exception.

Is this an expected behaviour given the sample application I provided?

Application code app.py:

import os

import sentry_sdk
from sanic import Sanic, response
from sentry_sdk.integrations.sanic import SanicIntegration

sentry_sdk.init(
    dsn=os.environ['SENTRY_DSN'], environment='dev', integrations=[SanicIntegration()], debug=True
)
app = Sanic(__name__)

class CustomException(Exception):
    pass

@app.route('/custom_error')
async def get(request):
    raise CustomException()

@app.exception(CustomException)
async def custom_error_handler(request, exception):
    # Exception has already been captured and sent to Sentry when we reach this point
    message = {'description': 'Custom Exception'}
    return response.json(message, status=401)

if __name__ == "__main__":
    app.run(host="0.0.0.0", port=8000, debug=True, auto_reload=False, workers=1)

Web server start command and logs:

$ python app.py
 [sentry] DEBUG: Setting up integrations (with default = True)
 [sentry] DEBUG: Setting up previously not enabled integration sanic
 [sentry] DEBUG: Setting up previously not enabled integration logging
 [sentry] DEBUG: Setting up previously not enabled integration stdlib
 [sentry] DEBUG: Setting up previously not enabled integration excepthook
 [sentry] DEBUG: Setting up previously not enabled integration dedupe
 [sentry] DEBUG: Setting up previously not enabled integration atexit
 [sentry] DEBUG: Setting up previously not enabled integration modules
 [sentry] DEBUG: Setting up previously not enabled integration argv
 [sentry] DEBUG: Setting up previously not enabled integration threading
 [sentry] DEBUG: Enabling integration sanic
 [sentry] DEBUG: Enabling integration logging
 [sentry] DEBUG: Enabling integration stdlib
 [sentry] DEBUG: Enabling integration excepthook
 [sentry] DEBUG: Enabling integration dedupe
 [sentry] DEBUG: Enabling integration atexit
 [sentry] DEBUG: Enabling integration modules
 [sentry] DEBUG: Enabling integration argv
 [sentry] DEBUG: Enabling integration threading
[2020-08-24 10:08:41 +0200] [7391] [DEBUG]

                 Sanic
         Build Fast. Run Fast.

[2020-08-24 10:08:41 +0200] [7391] [INFO] Goin' Fast @ http://0.0.0.0:8000
[2020-08-24 10:08:41 +0200] [7391] [INFO] Starting worker [7391]
[2020-08-24 10:08:47 +0200] - (sanic.access)[INFO][127.0.0.1:55202]: GET http://localhost:8000/custom_error  401 34
 [sentry] DEBUG: Sending event, type:null level:error event_id:<REDACTED> project:<REDACTED> host:<REDACTED>.ingest.sentry.io

curl command

$ curl -i http://localhost:8000/custom_error
HTTP/1.1 401 Unauthorized
Connection: keep-alive
Keep-Alive: 5
Content-Length: 34
Content-Type: application/json

{"description":"Custom Exception"}

pip freeze output

aiofiles==0.5.0
certifi==2020.6.20
chardet==3.0.4
click==7.1.2
h11==0.8.1
h2==3.2.0
hpack==3.0.0
hstspreload==2020.8.18
httptools==0.1.1
httpx==0.9.3
hyperframe==5.2.0
idna==2.10
multidict==4.7.6
pip-tools==5.3.1
rfc3986==1.4.0
sanic==19.12.2
sentry-sdk==0.16.5
six==1.15.0
sniffio==1.1.0
ujson==3.1.0
urllib3==1.25.10
uvloop==0.14.0
websockets==8.1
untitaker commented 4 years ago

This is currently intended behavior, but we could probably implement a similar solution to #303

simonlindroosfyndiq commented 4 years ago

Thank you for you answer, we did not expect this to be the expected behaviour. For now we will remove the SanicIntegration for our team to proceed.

untitaker commented 4 years ago

Sanic integration adds request data to your events so you'll lose some product value from disabling it. I'd recommend using the before_send hook to filter out CustomException: https://docs.sentry.io/error-reporting/configuration/filtering/?platform=python#before-send

Instead of setting a fingerprint you'd return None to drop the event.

j-boivie commented 3 years ago

I've come across this as well, and I'm a bit surprised that this is the intended behavior. The docs for the Sanic integration say:

All exceptions leading to an Internal Server Error are reported.

I took this to mean that all exceptions that result in a 500 visible to a user would be reported, but perhaps I misunderstood? We're using custom exception handlers to handle 4xx errors, that shouldn't be logged to Sentry (which I think is a fairly common pattern?), but this makes that difficult.

github-actions[bot] commented 2 years ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

antonpirker commented 2 years ago

Hello @simonlindroosfyndiq and @j-boivie ! Just a quick update: I think it would be good if we would have the behavior similar to the one described in https://github.com/getsentry/sentry-python/issues/303 like @untitaker suggested.

(so basically only send unhandled exceptions to Sentry)

So, anyone of you (or anyone else from the Sanic community) is motivated and also has the time to provide a PR for this? (In Sentry we will not be able to implement this in the next couple of months, I am afraid)

github-actions[bot] commented 2 years ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀