Description
This issue is a bit long, and cover two topics, to please read it all before
commenting.
1️⃣ Do not log anymore 4XX exceptions
With a fresh Symfony (5.1) but it's the same for older version, all exceptions
are logged. It could be are at error
level or at critical
level.
I created a simple controller:
<?php
namespace App\Controller;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Exception\BadRequestException;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\Routing\Annotation\Route;
class HomepageController extends AbstractController
{
/** @Route("/", name="homepage") */
public function index(): Response
{
throw new BadRequestException();
}
/** @Route("/2", name="homepage_2") */
public function index2(): Response
{
throw new BadRequestHttpException();
}
}
And when I hit the application, I got the following log (prod env)
[2020-10-30T15:52:01.636868+01:00] request.INFO: Matched route "homepage". {"route":"homepage","route_parameters":{"_route":"homepage","_controller":"App\\Controller\\HomepageController::index"},"request_uri":"https://127.0.0.1:8000/","method":"GET"} []
[2020-10-30T15:52:01.637921+01:00] request.ERROR: Uncaught PHP Exception Symfony\Component\HttpKernel\Exception\BadRequestHttpException: "" at /tmp/tmp/vendor/symfony/http-kernel/HttpKernel.php line 82 {"exception":"[object] (Symfony\\Component\\HttpKernel\\Exception\\BadRequestHttpException(code: 0): at /tmp/tmp/vendor/symfony/http-kernel/HttpKernel.php:82)\n[previous exception] [object] (Symfony\\Component\\HttpFoundation\\Exception\\BadRequestException(code: 0): at /tmp/tmp/src/Controller/HomepageController.php:16)"} []
[2020-10-30T15:52:04.005203+01:00] request.INFO: Matched route "homepage_2". {"route":"homepage_2","route_parameters":{"_route":"homepage_2","_controller":"App\\Controller\\HomepageController::index2"},"request_uri":"https://127.0.0.1:8000/2","method":"GET"} []
[2020-10-30T15:52:04.009468+01:00] request.ERROR: Uncaught PHP Exception Symfony\Component\HttpKernel\Exception\BadRequestHttpException: "" at /tmp/tmp/src/Controller/HomepageController.php line 22 {"exception":"[object] (Symfony\\Component\\HttpKernel\\Exception\\BadRequestHttpException(code: 0): at /tmp/tmp/src/Controller/HomepageController.php:22)"} []
For recall, from the monolog
documentation
ERROR (400): Runtime errors that do not require immediate action but should typically be logged and monitored.
So there is two things here:
- Symfony logged at the
error
level, meaning I should monitor this, and of
course, fix it. - My application decided to throw a 400. And 4XX are for user errors. This
means my code is good, the request is not. => There is nothing I can do to
fix it
This is mismatch to me.
This is even more obvious with 404's. This is why, in the past we introduced in
monolog the options excluded_404s
.
And then we also add excluded_http_codes
because, well, almost no one want
logs for 405, 403, 401, etc.
And there is also activation_strategy
to have full control. This is what I use
in my application for ages. And I copy this code from projets to projects. Boring
So my proposable it to not log at error
level HTTP exceptions < 500. We could
make it opt-in, but IMHO, this is a good default.
2️⃣ HttpException in the "model layer".
When we create controller, we have the power to manage exceptions handling, and
forge the response the use the status code we want.
But when we use third party code, like easy-admin, api-platform, etc, usually we
don't write the controller by ourself. So we can not really convert an exception
to a status code easily.
Api-platform offer us an option to convert exception instance to status code,
but I would like to go a bit further here.
First I'm not very confortable to throw *HttpException
from
symfony/http-kernel
in my model layer. It does not sens when I use theses service in my commands, or
workers.
I would feel more natural to have "raw" exception, without a binding to the SAPI.
Something like:
NotFoundException
ForbidenException
InvalidInputException
- etc
Symfony would, of course, convert a NotFoundException
to a 404 when needed.
And now, we have a nice new error handlers (with serializer), so we could
easily throw raw exceptions, and have a nice handling. APIP could even remove
it's own handling from its codebase. Like:
- https://github.com/api-platform/core/blob/master/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php
- https://github.com/api-platform/core/blob/master/src/EventListener/ExceptionListener.php
- etc
And we could also have a map of custom exception to status code (and a way to
enable log or not for such exception, see 1️⃣)
WDYT ?