Skip to content

fix(symfony): log the ErrorListener diagnostic at debug level - #8433

Open
davidmpaz wants to merge 1 commit into
api-platform:4.3from
davidmpaz:fix-error-listener-debug-log-level
Open

fix(symfony): log the ErrorListener diagnostic at debug level#8433
davidmpaz wants to merge 1 commit into
api-platform:4.3from
davidmpaz:fix-error-listener-debug-log-level

Conversation

@davidmpaz

Copy link
Copy Markdown
Q A
Branch? 4.3
Tickets Closes #8432
License MIT
Doc PR n/a

Changes ErrorListener::duplicateRequest() to log its diagnostic at debug level instead of error.

The message announces that the listener is converting an exception into an Error resource — normal operation, not a fault — and the call is already gated behind $this->debug.

The practical effect: without Monolog, Symfony falls back to HttpKernel\Log\Logger, and the phpunit/phpunit recipe in symfony/recipes ships SHELL_VERBOSITY=-1, which sets that logger's threshold to exactly error. So any functional test asserting a 4xx prints the line in the middle of an otherwise green suite:

............................................[error] An exception occured, transforming to an Error resource.
.....................  66 / 66 (100%)

OK (66 tests, 225 assertions)

which reads as a failure. It is also unreachable by framework.exceptions[].log_level, Symfony's own mechanism for reclassifying client errors, because the level is hardcoded here.

Test

testDiagnosticIsLoggedAtDebugLevel asserts debug() is called and error() is not. I verified it fails against the unpatched listener:

1) ApiPlatform\Symfony\Tests\EventListener\ErrorListenerTest::testDiagnosticIsLoggedAtDebugLevel
Psr\Log\LoggerInterface::error('An exception occured, transfo...ource.', [...]): void was not expected to be called.

and passes with it. src/Symfony/Tests/EventListener/ErrorListenerTest.php is green (5 tests, 34 assertions).

Notes

  • No BC concern that I can see: the message, its context array and the $this->debug guard are unchanged, only the severity.
  • php-cs-fixer reports nothing to fix on either file.
  • Running the full src/Symfony/Tests suite, one unrelated failure remains in ApiPlatformExtensionTest (api_platform.swagger_ui.processor). I confirmed it is pre-existing by re-running it on a pristine 4.3 checkout with my changes stashed.
  • I left the occuredoccurred typo alone to keep this to a single concern. Happy to add it here or in a separate PR, whichever you prefer.

The message announcing that an exception is being transformed into an
Error resource describes normal operation and is already gated behind
$debug, but it was emitted at `error` level.

Without Monolog, Symfony falls back to its stderr logger, and the
phpunit recipe ships SHELL_VERBOSITY=-1, which sets that logger's
threshold to exactly `error`. Any functional test asserting a 4xx
therefore prints the line in an otherwise green suite, where it reads
as a failure.

Closes api-platform#8432
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant