Should AbstractHandler::handle() return false when message is not handled and bubble is not allowed?

[ste93cry]

I have a question regarding both version 1.x and 2.x. In getsentry/sentry-php#1189 there is an open question about what is the general expected behaviour of the HandlerInterface::handle() method in regards to how its return value is computed. The question can be summarized in: should the $bubble argument of the constructor affect in any way the return value of that method? There are three distinct PHPDoc that are open to different interpretation:

monolog/src/Monolog/Handler/AbstractHandler.php

Lines 29 to 31 in c197198

	* @param bool $bubble Whether the messages that are handled can bubble up the stack or not
	*/
	public function __construct($level = Logger::DEBUG, bool $bubble = true)

Reading this statement, what I understand is that $bubble applies only to handled messages. In my case, if the event isn't sent off to the server for whatever reason, even if no error is really thrown, I would by definition say that the message has not been handled and I would expect it to be forwarded to the next handler of the chain regardless

monolog/src/Monolog/Handler/AbstractHandler.php

Lines 71 to 75 in c197198

	* @param bool $bubble true means that this handler allows bubbling.
	* false means that bubbling is not permitted.
	* @return self
	*/
	public function setBubble(bool $bubble): self

Reading this statement instead, I understand that the $bubble argument controls the bubbling regardless of whether the message has been handled or not. In my case, if the message failed to be sent and bubbling is disabled I would expect the message to not be passed to the next handler in the chain.

monolog/src/Monolog/Handler/HandlerInterface.php

Lines 47 to 50 in c197198

	* @return bool true means that this handler handled the record, and that bubbling is not permitted.
	* false means the record was either not processed or that this handler allows bubbling.
	*/
	public function handle(array $record): bool;

Finally, reading this statement I have no doubts about its meaning, but here arises the question: if $bubble is false and this method didn't handle the message, what should it return? The actual behaviour of all handlers I looked at is that the same condition is always used to determine the result value of the HandlerInterface::handle() method:

monolog/src/Monolog/Handler/AbstractProcessingHandler.php

Line 44 in c197198

return false === $this->bubble;

This condition satisfies only the second statement in the strict meaning.

[Seldaek]

The assumption is that handlers accept/handle a message and it'll be delievered. That is what the bubble was meant to do: If the message is handled (based on level usually), then it should false if bubbling is allowed, so other handlers keep being called. If the message is not handled, return false always (this is maybe what you missed, I believe it answers your last statement)

monolog/src/Monolog/Handler/AbstractProcessingHandler.php

Line 33 in c197198

return false;

If bubbling is not allowed, and the message is handled, then true is returned, and the handler chain is interrupted.

There was not a consideration of the handler looking at whether the message was properly delivered, because well this is usually not used as a message queue where losing data is life and death. That said I don't see this as a wrong interpretation to say that you consider a message handled only if it was acknowledged by the backend. Most handlers will probably throw if they fail to log something though so I don't think this applies to most things, but feel free to do it if it makes sense in your handler.

[ste93cry]

That is what the bubble was meant to do: If the message is handled (based on level usually), then it should false if bubbling is allowed, so other handlers keep being called. If the message is not handled, return false always

Ok so with $bubble = false in the constructor if the message has not been handled the expectation is that the next handler of the chain should still be called, right? This looks like a countersense though, because if I set in the constructor that I don't want bubbling I don't expect bubbling in any case.

[Seldaek]

Nope, as per:

If bubbling is not allowed, and the message is handled, then true is returned, and the handler chain is interrupted.

So $bubble = false would in monolog core always return true for messages which have a level that makes them "handled", regardless if the backend handled them properly or not.

That said I don't see this as a wrong interpretation to say that you consider a message handled only if it was acknowledged by the backend.

This is still true IMO. Yes it is a little counter intuitive to me that if I say no bubbling the handler would still bubble, but I can also see this as a valid use case.. Though it may be more fitting to have a fallback handler of some kind, that forwards to a secondary handler if the primary threw an exception (i.e. failed handling a message). Kind of like WhatFailureGroupHandler does but more like a FirstHandlerWinsGroupHandler where it'd just return after any handler successfully handled the record.

[juan-morales]

I believe that is a good chance think about wha handle() should return.

I also had this confusion at the beginning, did not make much sense the return value of handle() .

The main reason behind this relies on this piece of code:

file: Logger.php

code:

            try {
                if (true === $handler->handle($record)) {                    //<-------- THIS CODE
                    break;
                }
            } catch (Throwable $e) {
                $this->handleException($e, $record);

                return true;
            }

Is very old (year 2015).

I also "see" that we do have a method called getBubble() in Handler/AbstractHandler.php that is not used anywhere in the code except on some unit tests.

Maybe is a good chance to think about this, not for this version of monolog, but maybe in the next ones.