From 8c995756e04dee4896d358dc455adb3fcb712272 Mon Sep 17 00:00:00 2001 From: Aleksei Volkov Date: Mon, 5 Feb 2024 16:29:27 +0300 Subject: [PATCH] errors: more verbose error reporting in `StreamConnection` This patch makes error reporting in `read` and `send` methods of `StreamConnection` more verbose to provide aid in debugging network errors. --- src/Connection/StreamConnection.php | 21 ++++++++++----- src/Exception/CommunicationFailed.php | 6 +++++ tests/Integration/ClientMiddlewareTest.php | 2 +- .../Integration/Connection/ConnectionTest.php | 9 +++++-- .../Connection/ParseGreetingTest.php | 9 +++++-- tests/Integration/Connection/ReadTest.php | 27 ++++++++++++++----- 6 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/Connection/StreamConnection.php b/src/Connection/StreamConnection.php index 0202e55f..e859ea87 100644 --- a/src/Connection/StreamConnection.php +++ b/src/Connection/StreamConnection.php @@ -123,7 +123,7 @@ public function open() : Greeting return $this->greeting = Greeting::unknown(); } - $greeting = $this->read(Greeting::SIZE_BYTES, 'Unable to read greeting'); + $greeting = $this->read(Greeting::SIZE_BYTES, 'Error reading greeting'); return $this->greeting = Greeting::parse($greeting); } @@ -146,14 +146,18 @@ public function isClosed() : bool public function send(string $data) : string { - if (!$this->stream || !\fwrite($this->stream, $data)) { - throw new CommunicationFailed('Unable to write request'); + if (!$this->stream) { + throw new CommunicationFailed('Error writing request: connection closed'); } - $length = $this->read(PacketLength::SIZE_BYTES, 'Unable to read response length'); + if (!\fwrite($this->stream, $data)) { + throw CommunicationFailed::withLastPhpError("Error writing request"); + } + + $length = $this->read(PacketLength::SIZE_BYTES, 'Error reading response length'); $length = PacketLength::unpack($length); - return $this->read($length, 'Unable to read response'); + return $this->read($length, 'Error reading response'); } private function read(int $length, string $errorMessage) : string @@ -165,6 +169,11 @@ private function read(int $length, string $errorMessage) : string /** @psalm-suppress PossiblyNullArgument */ $meta = \stream_get_meta_data($this->stream); - throw new CommunicationFailed($meta['timed_out'] ? 'Read timed out' : $errorMessage); + if ($meta['timed_out']) { + throw new CommunicationFailed('Read timed out'); + } + + + throw CommunicationFailed::withLastPhpError($errorMessage); } } diff --git a/src/Exception/CommunicationFailed.php b/src/Exception/CommunicationFailed.php index 3769e04e..d0ae250a 100644 --- a/src/Exception/CommunicationFailed.php +++ b/src/Exception/CommunicationFailed.php @@ -15,4 +15,10 @@ final class CommunicationFailed extends \RuntimeException implements ClientException { + public static function withLastPhpError(string $errorMessage): self + { + $error = error_get_last(); + + return new self($error ? \sprintf("%s: %s", $errorMessage, $error['message']) : $errorMessage); + } } diff --git a/tests/Integration/ClientMiddlewareTest.php b/tests/Integration/ClientMiddlewareTest.php index 300caba2..4fcfaaa2 100644 --- a/tests/Integration/ClientMiddlewareTest.php +++ b/tests/Integration/ClientMiddlewareTest.php @@ -140,7 +140,7 @@ public function testThrowOnBrokenConnection() : void $client->ping(); $this->expectException(CommunicationFailed::class); - $this->expectExceptionMessage('Unable to write request'); + $this->expectExceptionMessage('Error writing request: fwrite(): Send of 15 bytes failed with errno=32 Broken pipe'); $client->ping(); } diff --git a/tests/Integration/Connection/ConnectionTest.php b/tests/Integration/Connection/ConnectionTest.php index 14031e69..222588c2 100644 --- a/tests/Integration/Connection/ConnectionTest.php +++ b/tests/Integration/Connection/ConnectionTest.php @@ -190,12 +190,13 @@ public function testUnexpectedResponse() : void public function testOpenConnectionHandlesTheMissingGreetingCorrectly() : void { $clientBuilder = ClientBuilder::createForFakeServer(); + $uri = $clientBuilder->getUri(); FakeServerBuilder::create( new AtConnectionHandler(1, new WriteHandler('')), new AtConnectionHandler(2, new WriteHandler(GreetingDataProvider::generateGreeting())) ) - ->setUri($clientBuilder->getUri()) + ->setUri($uri) ->start(); $client = $clientBuilder->build(); @@ -205,7 +206,11 @@ public function testOpenConnectionHandlesTheMissingGreetingCorrectly() : void $connection->open(); self::fail('Connection not established'); } catch (CommunicationFailed $e) { - self::assertSame('Unable to read greeting', $e->getMessage()); + self::assertSame( + sprintf('Error reading greeting: ' . + 'stream_socket_client(): Unable to connect to %s ' . + '(Connection refused)', $uri) + , $e->getMessage()); // At that point the connection was successfully established, // but the greeting message was not read } diff --git a/tests/Integration/Connection/ParseGreetingTest.php b/tests/Integration/Connection/ParseGreetingTest.php index 137dae2f..c7feb78f 100644 --- a/tests/Integration/Connection/ParseGreetingTest.php +++ b/tests/Integration/Connection/ParseGreetingTest.php @@ -27,9 +27,10 @@ final class ParseGreetingTest extends TestCase public function testParseGreetingWithInvalidServerName(string $greeting) : void { $clientBuilder = ClientBuilder::createForFakeServer(); + $uri = $clientBuilder->getUri(); FakeServerBuilder::create(new WriteHandler($greeting)) - ->setUri($clientBuilder->getUri()) + ->setUri($uri) ->start(); $client = $clientBuilder->build(); @@ -37,7 +38,11 @@ public function testParseGreetingWithInvalidServerName(string $greeting) : void try { $client->ping(); } catch (CommunicationFailed $e) { - self::assertSame('Unable to read greeting', $e->getMessage()); + self::assertSame( + sprintf("Error reading greeting: " . + "stream_socket_client(): Unable to connect to %s " . + "(Connection refused)", $uri), + $e->getMessage()); return; } catch (\RuntimeException $e) { diff --git a/tests/Integration/Connection/ReadTest.php b/tests/Integration/Connection/ReadTest.php index 6c3dd5c9..d8cfc947 100644 --- a/tests/Integration/Connection/ReadTest.php +++ b/tests/Integration/Connection/ReadTest.php @@ -35,15 +35,20 @@ public function testReadLargeResponse() : void public function testReadEmptyGreeting() : void { $clientBuilder = ClientBuilder::createForFakeServer(); + $uri = $clientBuilder->getUri(); FakeServerBuilder::create() - ->setUri($clientBuilder->getUri()) + ->setUri($uri) ->start(); $client = $clientBuilder->build(); $this->expectException(CommunicationFailed::class); - $this->expectExceptionMessage('Unable to read greeting'); + $this->expectExceptionMessage( + \sprintf('Error reading greeting: ' . + 'stream_socket_client(): Unable to connect to %s ' . + '(Connection refused)', $uri) + ); $client->ping(); } @@ -51,18 +56,23 @@ public function testReadEmptyGreeting() : void public function testUnableToReadResponseLength() : void { $clientBuilder = ClientBuilder::createForFakeServer(); + $uri = $clientBuilder->getUri(); FakeServerBuilder::create( new WriteHandler(GreetingDataProvider::generateGreeting()), new SleepHandler(1) ) - ->setUri($clientBuilder->getUri()) + ->setUri($uri) ->start(); $client = $clientBuilder->build(); $this->expectException(CommunicationFailed::class); - $this->expectExceptionMessage('Unable to read response length'); + $this->expectExceptionMessage( + \sprintf('Error reading response length: ' . + 'stream_socket_client(): Unable to connect to %s ' . + '(Connection refused)', $uri) + ); $client->ping(); } @@ -90,19 +100,24 @@ public function testReadResponseLengthTimedOut() : void public function testUnableToReadResponse() : void { $clientBuilder = ClientBuilder::createForFakeServer(); + $uri = $clientBuilder->getUri(); FakeServerBuilder::create( new WriteHandler(GreetingDataProvider::generateGreeting()), new WriteHandler(PacketLength::pack(42)), new SleepHandler(1) ) - ->setUri($clientBuilder->getUri()) + ->setUri($uri) ->start(); $client = $clientBuilder->build(); $this->expectException(CommunicationFailed::class); - $this->expectExceptionMessage('Unable to read response'); + $this->expectExceptionMessage( + \sprintf('Error reading response: ' . + 'stream_socket_client(): Unable to connect to %s ' . + '(Connection refused)', $uri) + ); $client->ping(); }