Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding events #206

Closed
wants to merge 14 commits into from
Closed

Adding events #206

wants to merge 14 commits into from

Conversation

jakubforman
Copy link

@jakubforman jakubforman commented Jun 1, 2024

Adds sending events that can be handled to within Event::listen().

Why I created this PR:

I am responding to #185 in which the need to track these events arose and PresenceChannel is insufficient for this.

Newly added events:

  • ClientConnected
  • ClientDisconnected
  • PusherSubscribe
  • PusherUnsubscribe

Each event is called similarly to the built-in original MessageReceived and MessageSent events.

The functionality can be extended using this PR as follows:

  • The ability to detect on the server Client Disconnects/Connections and respond to them.
  • Option to detect Unsubscribe/Subscribe from a channel and respond to them.
  • In the case of client disconnection (connection failure/window closing, ...) an PusherUnsubscribe event is also sent

☠️ Bug detected

When using the example below DeviceDisconnected event inside a Reverb server (tested on local), the server freezes if it is a synchronous send Queue. In the case of an asynchronous send (for example via DB) everything is fine.
This is related to #185 (comment)

Real Example

Showing one admin panel with many of devices (players) and their status.

  • Laravel API
  • Angular App (Admin - left)
  • Angular App (Player - right)

ezgif-2-0d4c0b7e80

Code sample and usage

Inside EventServiceProvider

<?php
// ...
class EventServiceProvider extends ServiceProvider
{
    // ...
    public function boot()
    {
        // Unsubscribe channel - offline state
        Event::listen( // swap player or auth info
            PusherUnsubscribe::class,
            DeviceDisconnectNotificationWS::class
        );
        // ...
    }
    // ...
}

Event Listener - Chnage in DB + Reverb Event Notification

<?php
namespace App\Listeners;

use App\Events\DeviceDisconnected;
use App\Models\Device;
use Illuminate\Support\Str;
use Laravel\Reverb\Events\PusherUnsubscribe;

class DeviceDisconnectNotificationWS
{
    public function handle(PusherUnsubscribe $event): void
    {
        $id = Str::after($event->channel, 'private-devices.');

        /** @var Device $device */
        $device = Device::find($id);
        if ($device) {
            $device->status = 'offline';
            $device->update();
            DeviceDisconnected::dispatch($device);
        }
    }
}

Custom Event DeviceDisconnected - called v DeviceDisconnectNotificationWS

<?php

namespace App\Events;

use App\Models\Device;
use Illuminate\Broadcasting\Channel;
use Illuminate\Broadcasting\InteractsWithSockets;
use Illuminate\Broadcasting\PrivateChannel;
use Illuminate\Contracts\Broadcasting\ShouldBroadcast;
use Illuminate\Foundation\Events\Dispatchable;
use Illuminate\Queue\SerializesModels;

class DeviceDisconnected implements ShouldBroadcast
{
    use Dispatchable;
    use InteractsWithSockets;
    use SerializesModels;

    public Device $device; // Hold status

    public function __construct(Device $device)
    {
        $this->device = $device;
    }

    public function broadcastWith()
    {
        return [
            "deviceId" => $this->device->id,
        ];
    }

    public function broadcastAs(): string
    {
        return 'status.disconnected';
    }

    public function broadcastOn(): array
    {
        return [
            new PrivateChannel('company.' . $this->device->company_id . '.devices'), // channel with authorization
        ];
    }
}

channels.php

Broadcast::channel('company.{companyId}.devices', function (User $user, string $companyId) {
    return $user->can('read', Company::find($companyId));
});

Broadcast::channel('devices.{deviceId}', function (User $user, string $deviceId) {
    return $user->can('read', Device::find($deviceId));
});

jakubforman and others added 14 commits May 30, 2024 17:34
The ClientDisconnected event has been created and is now dispatched upon a client's disconnection. This event is added to keep a record of all the channels that the client was subscribed to before the disconnect.
In pusher server, changed to convert channels objects into an array before they are passed into the ClientDisconnected dispatch function. This ensures the correct data type is supplied, preventing potential issues or errors.
The code has been refactored to allow the `ClientDisconnected` event to manage a single channel instead of multiple. The `close` function in the `Server` class has been updated to fetch the correct channel associated with the connection before triggering the `ClientDisconnected` event.
Handling has been added for null cases in the Pusher Server. The change includes adding conditions to check for null channel before extracting name and dispatching ClientDisconnected event. This prevents a potential crash when the channel doesn't exist.
The 'ClientDisconnected' event has been renamed to 'ChannelDisconnected' to better represent its functionality. This change was propagated across the codebase, particularly in the file 'src/Protocols/Pusher/Server.php', removing redundant usage of the 'ClientDisconnected' event. Now multiple 'ChannelDisconnected' events can be dispatched at once if multiple channels are disconnected simultaneously.
This update removes an unnecessary code block that assigns a channel name in the Pusher Server code. This was identified as superfluous since it doesn't impact the functioning of the subsequent unsubscribeFromAll method.
An outdated TODO comment within the Pusher protocol's "Server.php" file was removed. The comment was pertaining to the physical disconnection of multiple channels, which is no longer relevant or needed in the present implementation.
New events have been added to manage connection actions. This includes `PusherConnectionEstablished`, `PusherSubscribe`, and `PusherUnsubscribe` dispatched when a connection is established, a user subscribes, and a user unsubscribes respectively. This enhances the functionality of the Pusher server by providing more detailed and accurate events for the aforementioned actions.
The commit prioritizes the 'PusherUnsubscribe::dispatch()' operation over channel unsubscribe. Now, the 'PusherUnsubscribe::dispatch()' operation is invoked before unsubscribing from the channel. This ensures the dispatch operation is always run regardless of the success of channel unsubscription.
Renamed the ChannelDisconnected class to ClientDisconnected consistently across the codebase. This change reflects in the 'src/Events' and 'src/Protocols/Pusher' directories, improving code semantics to correlate to the actual functionality of the event - which is to manage client disconnections.
The specific class PusherConnectionEstablished has been renamed to ClientConnected to make it more general. The corresponding changes have also been made in the EventHandler class to properly dispatch the renamed event.
This commit modifies the disconnect function and event management flow to ensure channel unsubscription occurs prior to disconnection. The 'channel' parameter has been removed from the ClientDisconnected event constructor for this. Additional commenting has also been added to the ClientConnected and ClientDisconnected files to improve code understandability.
A comment regarding moving the dispatch function to the EventHandler was removed from Server.php. The TODO comment was stale, indicating a task which has likely already been completed or is no longer relevant in the current version of the project. The removal of this comment improves code cleanliness.
@joedixon
Copy link
Collaborator

joedixon commented Jun 3, 2024

Thank you for the pull request and the detailed explanation of why it is needed for your use case.

We're not adding new events to the project right now as they expose the possibility of blocking the event loop. For example, if the listener in your example is not queued, the event loop will be blocked since the message you are attempting to broadcast cannot be accepted until the listener execution completes and the listener execution cannot complete until the message has been broadcast. You end up in a deadlock.

@joedixon joedixon closed this Jun 3, 2024
@jakubforman
Copy link
Author

Thank you for the pull request and the detailed explanation of why it is needed for your use case.

We're not adding new events to the project right now as they expose the possibility of blocking the event loop. For example, if the listener in your example is not queued, the event loop will be blocked since the message you are attempting to broadcast cannot be accepted until the listener execution completes and the listener execution cannot complete until the message has been broadcast. You end up in a deadlock.

I understand the deadlock, it has happened to me when I use Queues in sync mode, that's why you need to have it as a Queue through an external service (SQS, DB...) - it hasn't happened there yet. Would there be any chance to get those events in there in the future? Alternatively, what would I have to do to make it possible to catch these events there.

@ah-rahimi
Copy link

Hello, I also need such events.
As far as I understand, does it mean that if the events are not queued, they will have problems?
Well, put a variable in the config file to turn on and off the events so that anyone who needs it can use it.
@joedixon

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