Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Let's talk about Exception and Log #38901

Copy link
Copy link
Closed
@lyrixx

Description

@lyrixx
Issue body actions

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:

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 ?

Metadata

Metadata

Assignees

No one assigned

    Labels

    FeatureMonologBridgeRFCRFC = Request For Comments (proposals about features that you want to be discussed)RFC = Request For Comments (proposals about features that you want to be discussed)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Morty Proxy This is a proxified and sanitized view of the page, visit original site.