Notice, that some channel transports already dispatches this event, appending to it some transport specific context.

If NotificationSender will dispatch it too, there will be two rised events.

@taylorotwell should I move the try/catch to each built-in channel/transport?

@Cellard

I see from the repos within the laravel-notification-channels org, that some repos dispatch the framework's NotificationFailed event, while others dispatch their own namespaced equivalent event.

https://github.com/search?q=org%3Alaravel-notification-channels%20NotificationFailed&type=code

While the ones which choose to dispatch a NotificationSent do so with their own namespaced equivalent event. None of those dispatch the framework's NotificationSent event.

https://github.com/search?q=org%3Alaravel-notification-channels%20NotificationSent&type=code

Although I would expect 3rd-party packages to dispatch their own events, I understand some of these packages are relying on this behavior for some time and changing this would be cumbersome for users relying on that behavior.

Maybe we can think of a solution to somehow throttle the framework's NotificationFailed event.

Perhaps registering a listener that shares some state with the NotificationSender instance, so we could tell if a Illuminate\Notifications\Events\NotificationFailed was already dispatched before dispatching it again.

But ideally, I believe the best would be to cooperate with 3rd-party packages and ask them to dispatch their own namespaced events.

The reasoning for my stance, is that monitoring tools like laravel/nightwatch are built around expectations about what the framework does, and often don't account for 3rd-party packages.

Such is that currently laravel/nightwatch has a TODO comment, assuming the NotificationFailed is never dispatched.

@taylorotwell

As noted on comment:

#55479 (comment)

Some community maintained channels already dispatch the Illuminate\Notifications\Events\NotificationFailed on their own upon failure to send a notification.

I believe it would be better if 3rd-party packages dispatched their own namespaced events, instead.

But considering some of these packages are doing this for some time already, I updated the PR to add a mechanism to prevent double dispatching the event.

I updated the PR's description with these changes.

On Laravel 13.x we could remove this hackish guard, and add a note on the upgrade guide asking 3rd-party packages to move to use their own namespaced events.

Excuse me, @rodrigopedra, but what if we have multichannel notification and few channels fail?

I think it would be better to prevent event duplication per channel (driver).

@Cellard, the guard only affects the top-level NotificationSender class.

It won't affect any channels already dispatching the event by themselves.

Let's say that inside the try block, 3 channels send a NotificationFailed event. That behavior was in place before the changes on this PR, and will keep the same after it.

Also, on the changes proposed here, the NotificationSender only dispatches a NotificationFailed event when an exception is caught, and no previous NotificationFailed was dispatched.

In a multichannel scenario, if an exception is thrown and not handled by a channel transport, it would already prevent, by the nature of an exception halting execution, other events to be dispatched by other channels, right?

The idea is not to deduplicate events channels already dispatch. Specially, as you pointed out in your comment, some channels might be adding their own data to the event's last parameter.

The guard was added to assure any previous behavior, regarding the NotificationFailed event, remains unchanged. If the previous behavior, in a multichannel configuration, was to have multiple NotificationFailed events dispatched, that behavior will keep the same.

The idea of the last changes is to prevent an additional NotificationFailed event to being dispatched on top of those. Or to dispatch an event only if none have been sent, so monitoring tools can hook up to that event.

Closed by mistake when syncing my local branch