[GH-ISSUE #1016] GelfHandler::close() leaves the handler in a broken state. #416

New issue

Closed

opened 2026-03-04 02:14:51 +03:00 by kerem · 0 comments

kerem commented

2026-03-04 02:14:51 +03:00

Owner

Originally created by @derula on GitHub (Jul 9, 2017).
Original GitHub issue: https://github.com/Seldaek/monolog/issues/1016

I commented on #614 because I was having the same problem, but my comment didn't get any visibility on a closed issue, so I'm hoping creating a new issue will help.

The gist is that in the destructor of Monolog\Handler\Handler, as well as in Monolog\ErrorHandler::handleFatalError(), ->close() is called on the handler. In case of GelfHandler, it unsets the publisher, leaving itself in a broken state.

If then some other code tries to log something, be it from a destructor that is randomly executed at a later time, or, in case of handleFatalError(), in another shutdown function, instead of gracefully refusing to log anything, the GelfHandler causes (another) fatal.

I agree that logging from shutdown functions or constructors may not be a good idea. However, I don't think the handler should cause a fatal error in these cases. For example, StreamHandler even re-opens the stream automatically in write() if it's been closed before. (I don't know whether that's a good idea, but that's what happens.)

Here is a short test script to show what I'm talking about:

<?php
require 'vendor/autoload.php';
$handler = new Monolog\Handler\GelfHandler(
    new Gelf\Publisher(new Gelf\Transport\UdpTransport())
);
$logger = new Monolog\Logger(null, [$handler]);
Monolog\ErrorHandler::register($logger);
register_shutdown_function(function() use ($logger) {
    $logger->debug('Something that shouldn\'t crash the application');
});
trigger_error('Something fatal', E_USER_ERROR);

Executing this will raise two fatal errors, first "something fatal," and then "Call to a member function publish() on null." For comparison, if you don't pass the handler to the Logger constructor, effectively creating a StreamHandler, only one fatal occurs (and afterwards the debug message is logged).

This can't be intended behavior...? Ideally, in my mind, every handler should act the same way and throw a specific exception if write() is called while it is closed. This exception could easily be caught and handled correctly. However, since for example the stream handler just gracefully re-opens its stream in that case, implementing this exception could be considered a BC break.

Originally created by @derula on GitHub (Jul 9, 2017). Original GitHub issue: https://github.com/Seldaek/monolog/issues/1016 I commented on #614 because I was having the same problem, but my comment didn't get any visibility on a closed issue, so I'm hoping creating a new issue will help. The gist is that in the destructor of Monolog\Handler\Handler, as well as in Monolog\ErrorHandler::handleFatalError(), ->close() is called on the handler. In case of GelfHandler, it unsets the publisher, leaving itself in a broken state. If then some other code tries to log something, be it from a destructor that is randomly executed at a later time, or, in case of handleFatalError(), in another shutdown function, instead of gracefully refusing to log anything, the GelfHandler causes (_another_) fatal. I agree that logging from shutdown functions or constructors may not be a good idea. However, I don't think the handler should cause a fatal error in these cases. For example, StreamHandler even re-opens the stream automatically in write() if it's been closed before. (I don't know whether that's a good _idea_, but that's what happens.) Here is a short test script to show what I'm talking about: <?php require 'vendor/autoload.php'; $handler = new Monolog\Handler\GelfHandler( new Gelf\Publisher(new Gelf\Transport\UdpTransport()) ); $logger = new Monolog\Logger(null, [$handler]); Monolog\ErrorHandler::register($logger); register_shutdown_function(function() use ($logger) { $logger->debug('Something that shouldn\'t crash the application'); }); trigger_error('Something fatal', E_USER_ERROR); Executing this will raise _two_ fatal errors, first "something fatal," and then "Call to a member function publish() on null." For comparison, if you don't pass the handler to the Logger constructor, effectively creating a StreamHandler, only one fatal occurs (and afterwards the debug message is logged). This can't be intended behavior...? Ideally, in my mind, every handler should act the same way and throw a specific exception if write() is called while it is closed. This exception could easily be caught and handled correctly. However, since for example the stream handler just gracefully re-opens its stream in that case, implementing this exception could be considered a BC break.