From 0eba38d4de1be96702d4edf9559e0deed719a3d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Thu, 12 Aug 2021 16:08:48 +0200 Subject: [PATCH 1/2] Improve error messages when accepting connections with errno/errstr --- src/TcpServer.php | 23 ++++++++++++++++++++++- tests/TcpServerTest.php | 22 +++++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/TcpServer.php b/src/TcpServer.php index 26eda8f7..185bba2b 100644 --- a/src/TcpServer.php +++ b/src/TcpServer.php @@ -213,7 +213,28 @@ public function resume() $this->loop->addReadStream($this->master, function ($master) use ($that) { $newSocket = @\stream_socket_accept($master, 0); if (false === $newSocket) { - $that->emit('error', array(new \RuntimeException('Error accepting new connection'))); + // Match errstr from PHP's warning message. + // stream_socket_accept(): accept failed: Connection timed out + $error = \error_get_last(); + $errstr = \preg_replace('#.*: #', '', $error['message']); + + // Go through list of possible error constants to find matching errno. + // @codeCoverageIgnoreStart + $errno = 0; + if (\function_exists('socket_strerror')) { + foreach (\get_defined_constants(false) as $name => $value) { + if (\strpos($name, 'SOCKET_E') === 0 && \socket_strerror($value) === $errstr) { + $errno = $value; + break; + } + } + } + // @codeCoverageIgnoreEnd + + $that->emit('error', array(new \RuntimeException( + 'Unable to accept new connection: ' . $errstr, + $errno + ))); return; } diff --git a/tests/TcpServerTest.php b/tests/TcpServerTest.php index 564614ca..97bcb241 100644 --- a/tests/TcpServerTest.php +++ b/tests/TcpServerTest.php @@ -287,7 +287,10 @@ public function testEmitsErrorWhenAcceptListenerFails() $server = new TcpServer(0, $loop); - $server->on('error', $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException'))); + $exception = null; + $server->on('error', function ($e) use (&$exception) { + $exception = $e; + }); $this->assertNotNull($listener); $socket = stream_socket_server('tcp://127.0.0.1:0'); @@ -297,6 +300,23 @@ public function testEmitsErrorWhenAcceptListenerFails() $time = microtime(true) - $time; $this->assertLessThan(1, $time); + + $this->assertInstanceOf('RuntimeException', $exception); + assert($exception instanceof \RuntimeException); + $this->assertStringStartsWith('Unable to accept new connection: ', $exception->getMessage()); + + return $exception; + } + + /** + * @param \RuntimeException $e + * @requires extension sockets + * @depends testEmitsErrorWhenAcceptListenerFails + */ + public function testEmitsTimeoutErrorWhenAcceptListenerFails(\RuntimeException $exception) + { + $this->assertEquals('Unable to accept new connection: ' . socket_strerror(SOCKET_ETIMEDOUT), $exception->getMessage()); + $this->assertEquals(SOCKET_ETIMEDOUT, $exception->getCode()); } public function testListenOnBusyPortThrows() From f10e66ed0486ae0209be0f0ecc61f46174103df9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 13 Aug 2021 09:27:24 +0200 Subject: [PATCH 2/2] Refactor to reuse connection error handling for Unix domain sockets --- src/SocketServer.php | 40 ++++++++++++++++++++++++++++++++++++++++ src/TcpServer.php | 29 ++++------------------------- src/UnixServer.php | 8 ++++---- tests/UnixServerTest.php | 22 +++++++++++++++++++++- 4 files changed, 69 insertions(+), 30 deletions(-) diff --git a/src/SocketServer.php b/src/SocketServer.php index 973bbaf8..88ae6487 100644 --- a/src/SocketServer.php +++ b/src/SocketServer.php @@ -90,4 +90,44 @@ public function close() { $this->server->close(); } + + /** + * [internal] Internal helper method to accept new connection from given server socket + * + * @param resource $socket server socket to accept connection from + * @return resource new client socket if any + * @throws \RuntimeException if accepting fails + * @internal + */ + public static function accept($socket) + { + $newSocket = @\stream_socket_accept($socket, 0); + + if (false === $newSocket) { + // Match errstr from PHP's warning message. + // stream_socket_accept(): accept failed: Connection timed out + $error = \error_get_last(); + $errstr = \preg_replace('#.*: #', '', $error['message']); + + // Go through list of possible error constants to find matching errno. + // @codeCoverageIgnoreStart + $errno = 0; + if (\function_exists('socket_strerror')) { + foreach (\get_defined_constants(false) as $name => $value) { + if (\strpos($name, 'SOCKET_E') === 0 && \socket_strerror($value) === $errstr) { + $errno = $value; + break; + } + } + } + // @codeCoverageIgnoreEnd + + throw new \RuntimeException( + 'Unable to accept new connection: ' . $errstr, + $errno + ); + } + + return $newSocket; + } } diff --git a/src/TcpServer.php b/src/TcpServer.php index 185bba2b..622d5575 100644 --- a/src/TcpServer.php +++ b/src/TcpServer.php @@ -211,31 +211,10 @@ public function resume() $that = $this; $this->loop->addReadStream($this->master, function ($master) use ($that) { - $newSocket = @\stream_socket_accept($master, 0); - if (false === $newSocket) { - // Match errstr from PHP's warning message. - // stream_socket_accept(): accept failed: Connection timed out - $error = \error_get_last(); - $errstr = \preg_replace('#.*: #', '', $error['message']); - - // Go through list of possible error constants to find matching errno. - // @codeCoverageIgnoreStart - $errno = 0; - if (\function_exists('socket_strerror')) { - foreach (\get_defined_constants(false) as $name => $value) { - if (\strpos($name, 'SOCKET_E') === 0 && \socket_strerror($value) === $errstr) { - $errno = $value; - break; - } - } - } - // @codeCoverageIgnoreEnd - - $that->emit('error', array(new \RuntimeException( - 'Unable to accept new connection: ' . $errstr, - $errno - ))); - + try { + $newSocket = SocketServer::accept($master); + } catch (\RuntimeException $e) { + $that->emit('error', array($e)); return; } $that->handleConnection($newSocket); diff --git a/src/UnixServer.php b/src/UnixServer.php index a3dd8a1a..25accbe0 100644 --- a/src/UnixServer.php +++ b/src/UnixServer.php @@ -113,10 +113,10 @@ public function resume() $that = $this; $this->loop->addReadStream($this->master, function ($master) use ($that) { - $newSocket = @\stream_socket_accept($master, 0); - if (false === $newSocket) { - $that->emit('error', array(new \RuntimeException('Error accepting new connection'))); - + try { + $newSocket = SocketServer::accept($master); + } catch (\RuntimeException $e) { + $that->emit('error', array($e)); return; } $that->handleConnection($newSocket); diff --git a/tests/UnixServerTest.php b/tests/UnixServerTest.php index 2e240933..0863be13 100644 --- a/tests/UnixServerTest.php +++ b/tests/UnixServerTest.php @@ -292,7 +292,10 @@ public function testEmitsErrorWhenAcceptListenerFails() $server = new UnixServer($this->getRandomSocketUri(), $loop); - $server->on('error', $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException'))); + $exception = null; + $server->on('error', function ($e) use (&$exception) { + $exception = $e; + }); $this->assertNotNull($listener); $socket = stream_socket_server('tcp://127.0.0.1:0'); @@ -302,6 +305,23 @@ public function testEmitsErrorWhenAcceptListenerFails() $time = microtime(true) - $time; $this->assertLessThan(1, $time); + + $this->assertInstanceOf('RuntimeException', $exception); + assert($exception instanceof \RuntimeException); + $this->assertStringStartsWith('Unable to accept new connection: ', $exception->getMessage()); + + return $exception; + } + + /** + * @param \RuntimeException $e + * @requires extension sockets + * @depends testEmitsErrorWhenAcceptListenerFails + */ + public function testEmitsTimeoutErrorWhenAcceptListenerFails(\RuntimeException $exception) + { + $this->assertEquals('Unable to accept new connection: ' . socket_strerror(SOCKET_ETIMEDOUT), $exception->getMessage()); + $this->assertEquals(SOCKET_ETIMEDOUT, $exception->getCode()); } public function testListenOnBusyPortThrows()