[MonologBridge] Fix interactive_only not preventing propagation#64207
[MonologBridge] Fix interactive_only not preventing propagation#64207nicolas-grekas merged 1 commit intosymfony:7.4symfony/symfony:7.4from philbates35:fix/monolog-bridge-interactive-only-propagationphilbates35/symfony:fix/monolog-bridge-interactive-only-propagationCopy head branch name to clipboard
interactive_only not preventing propagation#64207Conversation
004f21a to
be7bd8b
Compare
| self::assertTrue($handler->getBubble(), '->getBubble returns true when input is not interactive and interactiveOnly is true'); | ||
| } | ||
|
|
||
| public function testInteractiveOnlyPreventsPropagationWhenInteractive() |
There was a problem hiding this comment.
This fails without the fix, proving that propogation isn't stopped, which contradicts the docs and I believe the intended behaviour.
Runtime: PHP 8.2.31
Configuration: /home/runner/work/symfony/symfony/phpunit.xml
............................F.................................... 65 / 88 ( 73%)
....................... 88 / 88 (100%)
Time: 00:00.928, Memory: 24.00 MB
There was 1 failure:
1) Symfony\Bridge\Monolog\Tests\Handler\ConsoleHandlerTest::testInteractiveOnlyPreventsPropagationWhenInteractive
sibling handler must not receive records when interactive_only is true and input is interactive
Failed asserting that true is false.
/home/runner/work/symfony/symfony/src/Symfony/Bridge/Monolog/Tests/Handler/ConsoleHandlerTest.php:280
/home/runner/work/symfony/symfony/.phpunit/phpunit-11.5-0/phpunit:104
/home/runner/work/symfony/symfony/vendor/symfony/phpunit-bridge/bin/simple-phpunit.php:458
/home/runner/work/symfony/symfony/vendor/symfony/phpunit-bridge/bin/simple-phpunit:13
FAILURES!
Tests: 88, Assertions: 238, Failures: 1.
There was a problem hiding this comment.
Thanks for investigating this issue and providing a regression test.
As noted by @HypeMC, this should be fixed in Monolog's AbstractProcessingHandler
- return false === $this->bubble;
+ return !$this->getBubble();Once monolog is updated, the ConsoleHandler::handle() method override can be removed.
interactive_only not preventing propagation
Fix submitted here: Seldaek/monolog#2031 |
GromNaN
left a comment
There was a problem hiding this comment.
Thanks for the update. We will be able to merge this PR into 7.4 to fix the bug. Later, once Seldaek/monolog#2031 get released, we can increase the minimum required version of monolog/monolog and remove ConsoleHandler::handler in the next minor release of symfony/monolog-bridge.
There's just one small correction left to make to the comment, which no longer matches the code.
098c1d8 to
61c3ba3
Compare
|
Thank you @philbates35. |
The
interactive_onlyflag onConsoleHandleris documented to prevent propagation to sibling handlers when the input is interactive, but it only overridesgetBubble(). Monolog 3'sAbstractProcessingHandler::handle()reads the$bubbleproperty directly inside the propagation decision (return false === $this->bubble), so the dynamic value computed by the override never reachesLogger::addRecord's loop. As a result, when configured alongside a stream handler (e.g. JSON to stderr for log aggregation), interactive runs produced duplicate records.Mirror the dynamic
getBubble()value into$this->bubblefor the duration of thehandle()call so the property-based check used by Monolog honours interactive_only as documented. The previous value is restored via finally so subsequent records and external readers of getBubble() see the original configuration.The existing
testInteractiveOnlyunit asserts the method-level contract but never wires the handler into aLoggerwith a sibling, which is why this slipped through. Add two regression tests at the propagation level: one that asserts a siblingTestHandlerdoes NOT receive the record when input is interactive, and one that asserts it still DOES when input is non-interactive.