Skip to content

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

Closed

Description

@PeterBaker0

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

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

Activity

  1. moved this to Waiting for: Product Owner in GitHub Issues with 👀 2on Mar 15, 2024
  2. antonpirker commented on Mar 18, 2024

    @antonpirker
    Contributor

    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.

  3. moved this from Waiting for: Product Owner to No status in GitHub Issues with 👀 2on Mar 18, 2024
  4. syserr0r commented on Apr 18, 2024

    @syserr0r

    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?

  5. moved this to Waiting for: Product Owner in GitHub Issues with 👀 2on Apr 18, 2024
  6. PeterBaker0 commented on Apr 18, 2024

    @PeterBaker0
    Author

    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.

  7. szokeasaurusrex commented on Apr 22, 2024

    @szokeasaurusrex
    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!

  8. moved this from Waiting for: Product Owner to No status in GitHub Issues with 👀 2on Apr 22, 2024
  9. self-assigned this
    on Apr 23, 2024
  10. PeterBaker0 commented on Jun 6, 2024

    @PeterBaker0
    Author

    Thanks @sentrivana !!

  11. syserr0r commented on Jun 7, 2024

    @syserr0r

    Does this need to be raised for every integration (i.e. do I need to raise an issue for the Django implementation of this fix)?

  12. sentrivana commented on Jun 7, 2024

    @sentrivana
    Contributor

    Hey @syserr0r, we have an issue to follow up with more frameworks here: #3134 So it's mostly a matter of us finding some time to do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions