Please use constructor property promotion

Yes! A couple questions:

• as we target PHP 8.1, do we want to keepprivate + getters, or can we usepublic readonly instead? (and if targeting Symfony 7 and PHP 8.2, straightreadonly class with public promoted properties)
• does Symfony allow a mix of promoted and non-promoted, like this:

  finalclass HandlerFailureEventextends AbstractHandlerEvent{publicfunction__construct(Envelope$envelope,string$handlerName,public\Throwable$exception,    ) {parent::__construct($envelope,$handlerName);    }}

I've settled onpublic readonly as I could find other occurrences in the7.0 branch. I also used a mix of promoted and non-promoted inHandlerFailureEvent, as in the code sample above.

The name of the class does not make it clear, that it is dispatched before.

Maybe BeforeHandlerStartEvent?

I actually thought about this, but wanted to keep things organized by prefix, likeWorker* events. I don't mind though, what about justBeforeHandlerEvent?

I like it more, but lets see what the others say

ActuallySendMessageMiddlewareuses$eventDispatcher, so I guess it's better to keep it consistent here?

Same for the success case and remove the$e variable?

I don't mind dispatching the failure event in thecatch block, however I'm a bit reluctant about dispatching the success event in thetry block, as a failure in any listener/subscriber of this event would mark thehandler as failed.

That's why I kept things outside thetry/catch.

It would be great if the$handlerDescriptor is fully passed to this event, and that a subscriber can mutate it.

This would solve my use case that I tried to implement in:

• [Messenger] MakeHandleMessageMiddleware::callHandler protected #50980

It would allow event subscribers to:

• read everything from the HandlerDescriptor, so also the options
• set a new HandlerDescriptor that has a custom callable that does specific logic.

I'm all for adding the wholeHandlerDescriptor in the events, you're right that it opens up a whole lot of possibilities.

However, I have no idea of the potential consequences of live-replacing a handler, so I'll leave it to others to decide if making theHandlerDescriptor replaceable is the way to go!

@ro0NL@OskarStark What do you think?

I have to admit that I also don't know what consequences this can have

@ruudk Do you actually have a need to replace theHandlerDescriptor at the moment, or can we leave it for a later PR to maybe make it replaceable in the future?

Having it would be great, but maybe I can use the event system already to do what I want. I agree that we can later always decide to open this up.

Could we at least add the full HandlerDescriptor instead of only the name? That would make it a bit better.

Could we at least add the full HandlerDescriptor instead of only the name? That would make it a bit better.

Yes, definitely. I'll change this.

@ruudk events now have the fullHandlerDescriptor!

It would be great if a HandlerFailureEvent subscriber could silence an exception.

Right now, it's only possible to log them. But by always passing the exception to the HandlerFailureEvent and dispatching that, a subscriber could say "ignore this".

A use case could be to annotate#[IgnoreException(SomeTimeoutException::class)] on the handler. In case of an exception, a HandlerFailureEvent subscriber would see that the handler wants to ignore this exception, and it will silence it.

From Messenger perspective all will be fine again.

Related to

• [Messenger] MakeHandleMessageMiddleware::callHandler protected #50980

We could add aHandlerFailureEvent::ignoreException(bool $ignore) method indeed. I don't mind adding this feature, and again I'll leave it to others to decide if adding it is a good idea.

@ro0NL@OskarStark What do you think?

Also fine, but then we should let a subscriber mutate the handler descriptor. See my previous comment.

Do you mean no abstract? What aboutAbstractWorkerMessageEvent?