Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to avoid logging expected exceptions that are correctly converted to status codes by Api-Platform?

In some of routes for our Api-Platform project, exceptions are thrown for some common error conditions.

E.g. while calling POST /orders, NewOrderHandler can throw either of this two if appropriate:

  • NotEnoughStock
  • NotEnoughCredit

All these exceptions belong to a DomainException hierarchy.

These exceptions are correctly converted to status code 400 on the response by using the exception_to_status configuration, and the response includes the appropriate error message. So far so good.

exception_to_status:
    App\Order\NotEnoughStock: !php/const Symfony\Component\HttpFoundation\Response::HTTP_BAD_REQUEST
    App\Order\NotEnoughCredit: !php/const Symfony\Component\HttpFoundation\Response::HTTP_BAD_REQUEST

The only problem is that the exception is still logged as a CRITICAL error, being treated as an "uncaught exception". This gets logged even in production.

I would have expected that by being converted to a correct status code (e.g. !== 500) these exceptions would be treated as "handled", and thus would not pollute the logs.

Throwing exceptions from the handler is convenient, because it helps deal with transactionality and automatically generates the appropriate error response message. It works for web and console.

Shouldn't these transactions be treated as handled? Is it necessary to create another exception listener to deal with this? And if creating an exception listener, how to do it so it doesn't interfere with Api-Platform error normalization?

like image 754
yivi Avatar asked Apr 07 '20 09:04

yivi


3 Answers

There is a simple answer: handling an exception is not catching an exception.

Even if you convert your exception to a 400 error, your exception is still uncaught... that's why symfony logs it and this is done here.

If you don't want to log any DomainException, just override the logException() method in order to skip logging if it's an instance of DomainException.

Here is an exemple:

namespace App\EventListener;

use Symfony\Component\HttpKernel\EventListener\ErrorListener;

class ExceptionListener extends ErrorListener
{
    protected function logException(\Exception $exception, string $message): void
    {
        if ($exception instanceof DomainException) {
            return;
        }

        parent::logException($exception, $message);
    }
}

Finally you need to tell Symfony to use this class instead of the Symfony one. Since there is no class parameter for the exception_listener service definition I recommend to use a compiler pass in order to replace the class.

namespace App;

use App\EventListener\ExceptionListener;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

class OverrideServiceCompilerPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container)
    {
        $definition = $container->getDefinition('exception_listener');
        $definition->setClass(ExceptionListener::class);
    }
}

See Bundle override for more details.

Alternatively, just decorate the exception_listener service with your own and no compiler pass is needed:

App\EventListener\ExceptionListener:
        decorates: 'exception_listener' 
like image 172
rpg600 Avatar answered Oct 31 '22 22:10

rpg600


I tested it on a dummy application and I got:

Apr 11 21:36:11 |CRITI| REQUES Uncaught PHP Exception App\Exception\DomainException: "This is no more logged" at D:\www\campagne\src\DataPersister\StationDataPersister.php line 53 Apr 11 23:36:12 |WARN | SERVER POST (400) /api/stations

You could implement your own log activation strategy:

This code is based on the HttpCode activation strategy

namespace App\Log

use App\Exception\DomainException;
use Monolog\Handler\FingersCrossed\ErrorLevelActivationStrategy;
use Symfony\Component\HttpKernel\Exception\HttpException;

/**
 * Activation strategy for logs
 */
class LogActivationStrategy extends ErrorLevelActivationStrategy
{

    public function __construct()
    {
        parent::__construct('error');
    }

    public function isHandlerActivated(array $record): bool
    {
        $isActivated = parent::isHandlerActivated($record);
        if ($isActivated && isset($record['context']['exception'])) {
            $exception = $record['context']['exception'];

            // This is a domain exception, I don't log it
            return !$exception instanceof DomainException;

            // OR if code could be different from 400
            if ($exception instanceof DomainException) {
                // This is a domain exception 
                // You log it when status code is different from 400.
                return 400 !== $exception->getStatusCode();
            }
        }

        return $isActivated;
    }
}

We also need to tell Monolog to use our ActivationStrategy

monolog:
    handlers:
        main:
            type: fingers_crossed
            action_level: info
            handler: nested
            activation_strategy: App\Log\LogActivationStrategy 
        nested:
            type: stream
            path: "%kernel.logs_dir%/%kernel.environment%.log"
            level: info
       console:
            type: console
            process_psr_3_messages: false
            channels: ["!event", "!doctrine", "!console"]

Now my log contains only :

Apr 11 23:41:07 |WARN | SERVER POST (400) /api/stations

Like @yivi, I'm not in love of my solution, because each time application will try to log something, you loose time in this function... And this method do not change the log, it removes it.

like image 45
Alexandre Tranchant Avatar answered Oct 31 '22 21:10

Alexandre Tranchant


While in Monolog, when using the fingers_crossed log handler, will let you exclude from logging requests that respond with certain statuses, it will only do so if the exception is an instance of HttpException:

I got around this by implementing a subscriber to convert the exception into an BadRequestHttpException.

final class DomainToHttpExceptionSubscriber implements EventSubscriberInterface
{
    public static function getSubscribedEvents(): iterable
    {
        return [ KernelEvents::EXCEPTION => 'convertException'];
    }

    public function convertException(ExceptionEvent $event): void
    {
        $exception = $event->getThrowable();

        if ($exception instanceof DomainException) {
            $event->setThrowable(
                new BadRequestHttpException(
                    $exception->getMessage(),
                    $exception
                )
            );

        }
    }
}

This coupled with this monolog configuration does the trick:

monolog:
    handlers:
        fingers:
              type: fingers_crossed
              action_level: warning
              excluded_http_codes:
                - 404
                - 400

I got this from this answer on a GitHub issue. It works, but I'm not in love the solution. Hopefully some other answer will come to improve on this.

like image 42
yivi Avatar answered Oct 31 '22 22:10

yivi