-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[HttpKernel] the debug log processor must be a callable #52426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/Symfony/Component/HttpKernel/Log/DebugLoggerConfigurator.php
Outdated
Show resolved
Hide resolved
6987632 to
81baf53
Compare
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddDebugLogProcessorPass.php
Outdated
Show resolved
Hide resolved
|
re-reading the existing code I have updated this PR to only enforce the processor to be a callable and reverted the other changes |
| private ?\Closure $processor = null; | ||
|
|
||
| public function __construct(DebugLoggerInterface $processor, bool $enable = null) | ||
| public function __construct(callable $processor, bool $enable = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be ProcessorInterface|callable for monolog 3, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could change it that way, but it's not needed IMO as the ProcessorInterface does nothing more than enforcing the __invoke() method to be present: https://github.com/Seldaek/monolog/blob/main/src/Monolog/Processor/ProcessorInterface.php#L26
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh perfect then
|
Thank you @xabbuh. |
This PR was merged into the 6.4 branch. Discussion ---------- [HttpKernel] Fix DebugLoggerConfigurator | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | - | License | MIT Issue introduced in #52426: by turning the callable into a closure, we broke the `instanceof` check later in the class. Commits ------- 33721f5 [HttpKernel] Fix DebugLoggerConfigurator
A Monolog processor must be a callable, but a
DebugLoggerInterfaceimplementation is not necessarily a callable.