diff --git a/src/EventListener/ErrorLoggerListener.php b/src/EventListener/ErrorLoggerListener.php index 4c3670667..807a28fb2 100644 --- a/src/EventListener/ErrorLoggerListener.php +++ b/src/EventListener/ErrorLoggerListener.php @@ -4,6 +4,7 @@ namespace Overblog\GraphQLBundle\EventListener; +use GraphQL\Error\ClientAware; use GraphQL\Error\UserError; use Overblog\GraphQLBundle\Error\UserWarning; use Overblog\GraphQLBundle\Event\ErrorFormattingEvent; @@ -50,6 +51,17 @@ public function onErrorFormatting(ErrorFormattingEvent $event): void return; } + // A client-safe exception (e.g. ArgumentsValidationException) is a client + // fault, not an internal server error, so it must not be logged as CRITICAL. + // Handle it like a UserError: log its previous cause (if any) at ERROR. + if ($exception instanceof ClientAware && $exception->isClientSafe()) { + if ($exception->getPrevious()) { + $this->log($exception->getPrevious()); + } + + return; + } + $this->log($exception, LogLevel::CRITICAL); } diff --git a/tests/EventListener/ErrorLoggerListenerTest.php b/tests/EventListener/ErrorLoggerListenerTest.php index c374fa298..a78f58fdb 100644 --- a/tests/EventListener/ErrorLoggerListenerTest.php +++ b/tests/EventListener/ErrorLoggerListenerTest.php @@ -11,12 +11,16 @@ use Overblog\GraphQLBundle\Error\UserWarning; use Overblog\GraphQLBundle\Event\ErrorFormattingEvent; use Overblog\GraphQLBundle\EventListener\ErrorLoggerListener; +use Overblog\GraphQLBundle\Validator\Exception\ArgumentsValidationException; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; use Psr\Log\LogLevel; +use Symfony\Component\Validator\ConstraintViolationList; +use Symfony\Component\Validator\Validation; +use function class_exists; use function sprintf; final class ErrorLoggerListenerTest extends TestCase @@ -124,5 +128,29 @@ public static function onErrorFormattingDataProvider(): Generator ['exception' => $exception], ], ]; + + // The following cases exercise ArgumentsValidationException, which lives in + // the optional Symfony validator component; skip them when it is absent. + if (class_exists(Validation::class)) { + // A client-safe exception without a previous cause must NOT be logged + // (before the fix it was logged as CRITICAL). See #1193. + yield [ + new Error('Wrapped ClientSafe exception without previous', null, null, [], null, new ArgumentsValidationException(new ConstraintViolationList())), + fn (TestCase $test) => $test->never(), + [fn (TestCase $test) => $test->anything()], + ]; + + // A client-safe exception with a previous cause is logged at ERROR, + // like a UserError — not CRITICAL. See #1193. + yield [ + new Error('Wrapped ClientSafe exception with previous', null, null, [], null, new ArgumentsValidationException(new ConstraintViolationList(), $exception)), + fn (TestCase $test) => $test->once(), + [ + LogLevel::ERROR, + sprintf('[GraphQL] Exception: Ko![0] (caught throwable) at %s line %s.', __FILE__, $exception->getLine()), + ['exception' => $exception], + ], + ]; + } } }