Sentry-python: Integration with Pyramid tweens

[drnextgis]

The common case in Pyramid is to handle exception with tweens that seat atop of main Pyramid handling function. With a current integration approach each exception occurring in main function goes to sentry. Is there a way to track just those errors that were not handled in tweens?

[untitaker]

As a quick fix you can ignore errors by class with https://docs.sentry.io/error-reporting/configuration/filtering/?platform=python#before-send

As far as I can tell tweens are just like errorhandlers in Flask. In Flask, we have the following behavior:

• If you register an errorhandler for the exception (@app.errorhandler(Exception)), it will swallow the exception and it will not report to Sentry
• If you register an errorhandler for a status code (@app.errorhandler(500)), we consider the errorhandler to be just a "pretty error page" and still report the error to sentry.

I wonder if we can figure out a rule like that for Pyramid too. I only had a look at exception views and was not sure if there is a way to distinguish those two cases, so I opted for what I felt was "safer", which is reporting to sentry

[rouge8]

I wonder if we can figure out a rule like that for Pyramid too. I only had a look at exception views and was not sure if there is a way to distinguish those two cases, so I opted for what I felt was "safer", which is reporting to sentry

We're seeing a similar issue where exception views for 4xx exceptions are now being sent to Sentry, e.g.:

class OurCustomError(Exception):
    pass


@view_config(context=OurCustomError)
def handle_our_custom_error(request):
    request.response.status_int = 404
    return {}

It looks like the current Pyramid integration is recording the exception before the handler is even called. 😞

[drnextgis]

@untitaker what do you think about implementing Pyramid integration as a tween in a same way it is done for pyramid_exclog?

[rouge8]

Or maybe making the Router.handle_request portion of the Pyramid integration optional, so we can explicitly log to Sentry in our exception handling views. 😕

[drnextgis]

@rouge8 current Pyramid integration lives inside a main handler but I think it should be placed above pyramid.tweens.excview_tween_factory. In that case you don't need to record exceptions in exception views explicitly.

[untitaker]

@drnextgis I don't want to make the user explicitly wire up the pyramid integration. I don't understand your suggestion in the sense that I don't know how it would affect behavior. We still have the problem that we don't know which exceptions are "handled" and which are "suppressed".

To be clear, I am worried about a catch-all exception view:

@view_config(context=Exception)
def foo(...):
    ...

Is the intention to make a default 500 error page or to really suppress all exceptions? As far as i can tell pyramid_exclog behaves the exact same way as the Sentry integration:

• Ignores HTTP exceptions
• Logs everything, even if a matching exception view exists
• Has separate config to ignore exception types (see link I posted at beginning of this thread)

(this is just what I read from the docs you linked)

[goodspark]

The intent of catch-all exceptions views are to have a custom 500 error page that doesn't mismatch the design of the site and/or conforms to API error schemas.

When a pyramid user has defined any exception-based view, I think it's reasonable to expect them to do all error handling themselves, including error reporting.

This seems to be the only reasonable alternative at the moment. I'd rather not be carpet bombed with every single 4xx and 5xx error we raise as an exception and manually add them to Sentry filter lists. When we upgraded to the latest SDK our issues dashboard exploded with a lot of non-errors and is now even noisier than before.

[mmerickel]

It seems to me that sentry should only log unhandled exceptions that propagate out of the app. If people register exception views to deal with exceptions then it feels reasonable that they will sentry_sdk.capture_exception themselves if they wish. The simplest way to do this is monkeypatching just the router.__call__ if that's possible.

[untitaker]

I'll change this behavior to what you all suggested in 0.9.0

[untitaker]

One quick question @goodspark, you don't see subtypes of HTTPException being reported to Sentry by any chance? That would be a hard bug as we intend to explicitly ignore those.

[untitaker]

What do you think about this behavior: Capture exception if there's no errorhandler, or if the errorhandler returned a response with status code 500

[rouge8]

What do you think about this behavior: Capture exception if there's no errorhandler, _or_ if the errorhandler returned a response with status code 500

That makes sense to me.

[untitaker]

Thanks for your patience, I'll release 0.9.0 next week.

[untitaker]

0.9.0 is released

[drnextgis]

Thanks! But amongst other test I'd like to have the following one (in test_pyramid.py):

def tween_factory(handler, registry):
    def tween(request):
        try:
            response = handler(request)
        except:
            mroute = request.matched_route
            if mroute and mroute.name in ("index",):
                status_code = 400
                return Response(
                    text=json.dumps(dict()),
                    status_code=status_code,
                    content_type="application/json",
                )
        return response
    return tween


def test_tween_ok(
    sentry_init,
    pyramid_config,
    capture_exceptions,
    route,
    get_client,
):
    sentry_init(integrations=[PyramidIntegration()])
    errors = capture_exceptions()

    @route("/")
    def index(request):
        raise Exception()

    pyramid_config.add_tween(
        "tests.integrations.pyramid.test_pyramid.tween_factory",
        under=pyramid.tweens.INGRESS
    )

    client = get_client()
    client.get("/")

    assert not errors

[untitaker]

ok, I'll add that.