Skip to content

Conversation

@rodrigopedra
Copy link
Contributor

PR #55507, authored by me, added logic to ensure the Illuminate\Notifications\Events\NotificationFailed is dispatched whenever a notification failed and also guard against double sending the same event in case a 3rd-party channel was already manually dispatching that event.

As noted by discussion #58434, the Illuminate\Notifications\ChannelManager always new up a new instance of the Illuminate\Notifications\NotificationSender class, and as such, a new listener for the Illuminate\Notifications\Events\NotificationFailed is registered times for each instance.

In a case where multiple notifications are sent, but then down the road one fails, those multiple listeners were invoked.

The current approach also prevents garbage collection of the Illuminate\Notifications\NotificationSender instances, as the registered listener would prevent those instances from being collected/destructed.

In a scenario where an artisan command is scheduled to send multiple notifications for several users, that could lead to a memory overflow.

This PR

  • Keeps a single instance of Illuminate\Notifications\NotificationSender on the Illuminate\Notifications\ChannelManager.
  • Adds a test case to ensure the listener registered by the Illuminate\Notifications\NotificationSender instance is just called once even after multiple notifications were sent.

Notes:

  • As Illuminate\Notifications\ChannelManager is already registered as a singleton, that will only instantiate a single Illuminate\Notifications\NotificationSender instance per life cycle.
  • With a single Illuminate\Notifications\NotificationSender instance, only one listener is reg

Thanks @macropay-solutions for the heads-up.

@taylorotwell taylorotwell merged commit b9a1c92 into laravel:12.x Jan 21, 2026
1 check failed
@rodrigopedra rodrigopedra deleted the feature-notification-sender branch January 21, 2026 15:55
@rodrigopedra
Copy link
Contributor Author

@taylorotwell

Should we remove the guard logic (listener, instance variable) from the NotificationSender on 13.x?

And request 3rd-party channel drivers to not dispatch the NotificationFailed event themselves?

If so, I can send a PR.

@BertvanHoekelen
Copy link
Contributor

This change breaks in Laravel Octane on the second request when the notification has the ShouldQueue interface. I get Target [Illuminate\Contracts\Queue\Factory] is not instantiable. on the second request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants