diff --git a/src/EventListener/ErrorLoggerListener.php b/src/EventListener/ErrorLoggerListener.php index 4c3670667..998c840e5 100644 --- a/src/EventListener/ErrorLoggerListener.php +++ b/src/EventListener/ErrorLoggerListener.php @@ -7,6 +7,7 @@ use GraphQL\Error\UserError; use Overblog\GraphQLBundle\Error\UserWarning; use Overblog\GraphQLBundle\Event\ErrorFormattingEvent; +use Overblog\GraphQLBundle\Validator\Exception\ArgumentsValidationException; use Psr\Log\LoggerInterface; use Psr\Log\LogLevel; use Psr\Log\NullLogger; @@ -50,6 +51,15 @@ public function onErrorFormatting(ErrorFormattingEvent $event): void return; } + // Validation failures are client faults, so handle them like UserError. + if ($exception instanceof ArgumentsValidationException) { + 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..978fad7d5 100644 --- a/tests/EventListener/ErrorLoggerListenerTest.php +++ b/tests/EventListener/ErrorLoggerListenerTest.php @@ -6,21 +6,28 @@ use Exception; use Generator; +use GraphQL\Error\ClientAware; use GraphQL\Error\Error; use Overblog\GraphQLBundle\Error\UserError; 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 { + private const CLIENT_SAFE_INTERNAL_MESSAGE = 'Safe internal error'; + private ErrorLoggerListener $listener; /** @@ -60,6 +67,12 @@ public function testOnErrorFormatting(Error $error, $expectedLoggerCalls, array public static function onErrorFormattingDataProvider(): Generator { $exception = new Exception('Ko!'); + $clientSafeInternalException = new class(self::CLIENT_SAFE_INTERNAL_MESSAGE) extends Exception implements ClientAware { + public function isClientSafe(): bool + { + return true; + } + }; yield [ new Error('Basic error'), @@ -95,6 +108,23 @@ public static function onErrorFormattingDataProvider(): Generator ], ]; + yield [ + new Error('Wrapped client-safe internal exception', null, null, [], null, $clientSafeInternalException), + fn (TestCase $test) => $test->once(), + [ + LogLevel::CRITICAL, + sprintf( + '[GraphQL] %s: %s[%d] (caught throwable) at %s line %s.', + $clientSafeInternalException::class, + self::CLIENT_SAFE_INTERNAL_MESSAGE, + $clientSafeInternalException->getCode(), + __FILE__, + $clientSafeInternalException->getLine() + ), + ['exception' => $clientSafeInternalException], + ], + ]; + yield [ new Error('Wrapped Base UserError with previous', null, null, [], null, new UserError('User error message', 0, $exception)), fn (TestCase $test) => $test->once(), @@ -124,5 +154,29 @@ public static function onErrorFormattingDataProvider(): Generator ['exception' => $exception], ], ]; + + // ArgumentsValidationException requires the optional Symfony validator + // component, so skip these cases when it is absent. + if (class_exists(Validation::class)) { + // Argument validation without a previous cause must NOT be logged + // (before the fix it was logged as CRITICAL). See #1193. + yield [ + new Error('Wrapped ArgumentsValidationException without previous', null, null, [], null, new ArgumentsValidationException(new ConstraintViolationList())), + fn (TestCase $test) => $test->never(), + [fn (TestCase $test) => $test->anything()], + ]; + + // Argument validation with a previous cause is logged at ERROR, + // like a UserError — not CRITICAL. See #1193. + yield [ + new Error('Wrapped ArgumentsValidationException 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], + ], + ]; + } } }