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.

Sign up for GitHub

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

Jump to bottom

Enable configuration of reported status codes for FastAPI/Starlette HTTPExceptions #2825

Open
PeterBaker0 opened this issue Mar 15, 2024 · 4 comments · May be fixed by #3008
Open

Enable configuration of reported status codes for FastAPI/Starlette HTTPExceptions #2825

PeterBaker0 opened this issue Mar 15, 2024 · 4 comments · May be fixed by #3008
Assignees
@sentrivana
Labels
enhancement New feature or request

Comments

@PeterBaker0
Copy link

PeterBaker0 commented Mar 15, 2024 •
edited

Problem Statement

When instrumenting a FastAPI application with Sentry, I want to be able to configure the status codes for which a HTTPException is reported to Sentry, so that I can control what kinds of errors are reported in sentry in a way which makes sense in my application.

Solution Brainstorm

The code which informs the status code filtering appears to be here

sentry-python/sentry_sdk/integrations/starlette.py

Line 216 in 16d25e2

and exp.status_code >= 500 

 async def _sentry_patched_exception_handler(self, *args, **kwargs):
                # type: (Any, Any, Any) -> None
                exp = args[0]

                is_http_server_error = (
                    hasattr(exp, "status_code")
                    and isinstance(exp.status_code, int)
                    and exp.status_code >= 500
                )
                # ...

I don't know enough about the sentry SDK/integration architecture to understand where/how/if integration specific config can be injected to this context.

I'd assume that if this is possible, it would be an easy addition.

A (rough) interface like so might be helpful:

from typing import Callable

# This function takes the status code and returns True iff should be reported
StatusCodeCheckFunction  = Callable[[int], bool]

# Then can setup your preferred check function 
def check_function(status_code:int)-> bool:
    range_min
    return status_code == 401 or status_code > 500 or status_code in [429, 403]

# then include in sentry init somehow
sentry_init(
#... other stuff
fast_api_config=FastApiConfig(check_function=check_function)
#... other stuff
@PeterBaker0 PeterBaker0 added the enhancement New feature or request label Mar 15, 2024
@antonpirker
Copy link
Member

Hey @PeterBaker0 thanks for writing in.

The Http status codes in the 4xx range are client side errors, this is why we do not sent them to Sentry. They should be captured and sent by the client of the FastAPI api.

@syserr0r
Copy link

Hey @PeterBaker0 thanks for writing in.

The Http status codes in the 4xx range are client side errors, this is why we do not sent them to Sentry. They should be captured and sent by the client of the FastAPI api.

I'm interested in 4xx client-side errors being reported in sentry (in my case in Django) and I've seen this response a few times, but yet I also see the exact set of options I desire in the Sentry docs for ASP.Net Core: https://docs.sentry.io/platforms/dotnet/guides/aspnetcore/configuration/http-client-errors/

Can you help me understand why there is a difference?

@PeterBaker0
Copy link
Author

PeterBaker0 commented Apr 18, 2024 •
edited

Good catch @syserr0r

I kind of get the logic @antonpirker - that's why defaults are provided - but not everything fits into a neat box.

I'm sure there are use cases where the client application isn't wanting to setup/use sentry reporting e.g. programmatic/headless clients? I think it's good to have the options - especially considering there is precedent in other clients as @syserr0r mentioned.

For example, what if you wanted to log all 401 errors in your API for security auditing purposes? A malicious client wouldn't nicely call the sentry API for you to report the error.

@szokeasaurusrex
Copy link
Member

Hey @PeterBaker0 and @syserr0r, thanks for bringing up the good points. It looks like this is a feature we should be supporting per our Develop documentation, so we will place this issue on our backlog.

Thanks for reporting!

@sentrivana sentrivana self-assigned this Apr 23, 2024
@sentrivana sentrivana linked a pull request Apr 24, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sentrivana sentrivana

Labels
enhancement New feature or request
Projects
GitHub Issues with 👀 2
Status: No status
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(integrations): Allow to configure status codes to report to Sentry getsentry/sentry-python
5 participants
@antonpirker @syserr0r @szokeasaurusrex @PeterBaker0 @sentrivana