getsentry / sentry-symfony

The official Symfony SDK for Sentry (sentry.io)
https://sentry.io
MIT License
695 stars 171 forks source link

NotFoundHttpException 's are not caught #128

Closed lashae closed 6 years ago

lashae commented 6 years ago

As the title implies, when user hits a nonexisting URL logs show that an exception of type Symfony\Component\HttpKernel\Exception\NotFoundHttpException is raised. However this is not caught/detected by this bundle.

Is this a bug, or a feature?

Jean85 commented 6 years ago

NotFoundHttpException are handled internally by Symfony to render a 404 page; it's normal that those are not handled the same because otherwise you would capture a lot of "noise", like spiders trying to crawl your site.

lashae commented 6 years ago

So can we conclude that:

Any exception extending Symfony\Component\HttpKernel\Exception will be catched by Symfony and this bundle won't act upon?

Jean85 commented 6 years ago

No, that's not what we do. This bundle listens to the kernel.exception event, and also attaches the Sentry SDK with error/exception handlers; so what we intercept is everything that bubbles up outside of the framework.

I'm not sure how, but the Symfony framework will not make a 404 bubble up as kernel.exception event, so if you really want to catch them you will have to extend the ExceptionListener of this bundle, and listen to some additional event.

lashae commented 6 years ago

@Jean85, I made a POST request to an endpoint which is configured to just accept GET requests. This yield to the following:

May  7 08:56:07 staging-api-server [10595]: request.ERROR: Uncaught PHP Exception
Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException: "No route found for 
"POST /secure/shift/start": Method Not Allowed (Allow: GET)" at 
/var/www/releases/20180502113520/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/EventListener/RouterListener.php 
line 148 {"exception":"[object] (Symfony\\Component\\HttpKernel\\Exception\\MethodNotAllowedHttpException(code: 0): 
No route found for \"POST /secure/shift/start\": Method Not Allowed (Allow: GET) at /var/www/releases/20180502113520/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/EventListener/RouterListener.php:148, Symfony\\Component\\Routing\\Exception\\MethodNotAllowedException(code: 0):  at /var/www/releases/20180502113520/var/cache/prod/appProdProjectContainerUrlMatcher.php:301)"} []

Which is in deed a MethodNotAllowedHttpException and this isn't catched by this plugin as well. Is this behaviour expected?

I suppose same is true for all Exceptions which are extending HttpException that's so I asked on my previous comment.

Jean85 commented 6 years ago

I understood your question but I don't think I'm currently able to answer it; you're asking me about internal Symfony behavior, you will have to dig in a little inside Symfony to check which exceptions are handled this way.

lashae commented 6 years ago

So let me sum this up: If a kernel.exception event is triggered it is caught by this plugin. No other event is listened by default. Is this correct?

Jean85 commented 6 years ago

Yes, that's correct. Other stuff that gets catched are fatal and other hard errors that interrupts the app flow abruptly (like a memory exhausted error).

cedricverhaeghe commented 5 years ago

It seems to me that @lashae was correct in his assessment that all Symfony exceptions extending the HttpExceptionInterface are ignored by default in this bundle.

This is configured here: code And checked for in the exception listener here: code

So a 404 and other http exceptions do trigger a kernel.exception in Symfony, but this module ignores them. However, this seems to be configurable by the skip_capture property.

rvanlaak commented 5 years ago

As a follow-up on this, it seems since Guard that the \Symfony\Component\Security\Core\Exception\AuthenticationException is used to start the authentication flow.

Given that exception does not extend/implement HttpExceptionInterface, Sentry starts receiving many issues.

For us this seems to be a regression that started happening when updating from v2.0.1 to v2.2.4

This could quickly be fixed by adding interfaces to skip_capture ? Or, [since it was removed in v3.x of ](

Jean85 commented 5 years ago

With the newer bundle (3.x) you get the newer version of the SDK (sentry/sentry 2.x) and that's why you no longer have skip_capture; you should use the excluded_exceptions SDK option instead.