amphp / websocket-server

WebSocket component for PHP based on the Amp HTTP server.
MIT License
114 stars 17 forks source link

doing something at client disconnect (onClose) and maybe phpdoc to update? #18

Closed DrLightman closed 1 year ago

DrLightman commented 4 years ago

Hello,

trying to learn this async stuff by extending the example in the readme to make a small game, going quite ok so far.

Now in the server I wanted to detect the disconnection of one client in order to remove all the player's data from the server so I went for a callback to pass to the $client's onClose() method:

use Amp\Http\Server\Driver\RemoteClient; // added for onClose(...)

// ...

public function handleHandshake(Request $request, Response $response): Promise
{
    global $logger;

    $client = $request->getClient(); // Amp\Http\Server\Driver\RemoteClient

    // ... stuff ...
    // ... stuff ...
    // ... stuff ...

    $client->onClose(function (RemoteClient $remoteClient) use ($logger) {
        $logger->warning(sprintf('CLOSED'));
    });

    return new Success($response);
}

// ...

Is that the right way to go? It seems to work but I don't know if it's the right optimal way :)

Also, the phpdoc of Amp\Websocket\Client onClose interface says:

/**
 * Attaches a callback invoked when the client closes. The callback is passed this object as the first parameter,
 * the close code as the second parameter, and the close reason as the third parameter.
 *
 * @param callable(Client $client, int $code, string $reason) $callback
 */

But it seems only one parameter is passed instead, the first. I get a Fatal Error if I include $code and $reason. In the implementation of Amp\Http\Server\Driver\RemoteClient's close():

        foreach ($onClose as $callback) {
            Promise\rethrow(call($callback, $this));
        }
elovin commented 4 years ago

@DrLightman I think you imported the wrong Client class, you should use the Amp\Websocket\Client or Amp\Websocket\Rfc6455Client which is also what the concrete ClientFactory in this repository returns.

DrLightman commented 4 years ago

@elovin Indeed the difference in the class was was exactly the reason.

Because the $client returned by $request->getClient() inside ::handleHandshake of the example in the readme.md is an Amp\Http\Server\Driver\RemoteClient, whereas the $client passed as parameter of ::handleClient is an Amp\Websocket\Client (or Amp\Websocket\Rfc6455Client as var_dump reports at runtime) and that provides the close callback with the three parameters.

kelunik commented 4 years ago

I guess we might need to improve the naming here.