Throw client read timeout earlier and don’t close connection #140
Conversation
|
I'm happy to update this to put this behaviour behind an option if necessary |
|
Hi @marcroberts , and thanks for contributing! I have run some manual tests on this PR, and it appears to have an issue. What I did was to set a low timeout on client, and then a sleep() of higher value on the echoserver to provoke timeout fails in client. I then end up with the "Empty read; connection dead?" error instead. As far as I can tell, the There's a comment in docs page that support this finding; https://www.php.net/manual/en/function.fread.php#115340 Besides this issue, I find your PR agreeable. |
|
That is curious. I'm using my branch in a staging environment at the moment and it is behaving as expected returning FALSE when the Here's my debug output when I set the client timeout to 5 seconds. I'm catching the TimeoutException and sending a PING Perhaps we should check for the timeout specifically before the FALSE / '' checks ? |
|
Have run some additional tests, but I get the same results as before. These are the test files I'm running; // File: test-server.php
<?php
namespace WebSocket;
require __DIR__ . '/vendor/autoload.php';
error_reporting(-1);
$server = new Server();
$server->setLogger(new EchoLog());
try {
$server->accept();
while (true) {
$message = $server->receive();
if (is_null($message)) exit; // Done
echo "Server received $message \n";
sleep(2);
$server->text("Server responding on: $message");
}
} catch (ConnectionException $e) {
echo "> ERROR: {$e->getMessage()}\n";
}// File: test-client.php
<?php
namespace WebSocket;
require __DIR__ . '/vendor/autoload.php';
error_reporting(-1);
try {
$client = new Client('ws://localhost:8000', ['timeout' => 1]);
$client->setLogger(new EchoLog());
$client->text('Client message');
$response = $client->receive();
echo "Client received: $response \n";
$client->close();
} catch (\Throwable $e) {
echo "ERROR: {$e->getMessage()}\n";
}Note the timeout of client is The output of client is now; If I change so it checks empty string, I instead get; Comparing to your log, above runs without SSL on local server. Maybe that's what causes the different results? But yes, I suppose the stable solution is to check for timeout on falsish results, before the more explicit null/empty string checks. |
|
That is odd. I'll update the PR to check for a timeout on falsey response. |
PR to solve issue #139
When a timeout occurs on the client-side in the
$client->receive()call this throws a TimeoutException earlier and keeps the connection open. This allows a PING to be sent to keep the connection alive if needed by the server.