Skip to content

Commit

Permalink
errors: more verbose error reporting in StreamConnection
Browse files Browse the repository at this point in the history
This patch makes error reporting in `read` and `send` methods of
`StreamConnection` more verbose to provide aid in debugging network
errors.
  • Loading branch information
AlgebraicWolf authored and rybakit committed Feb 7, 2024
1 parent 5d949c3 commit 8c99575
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 17 deletions.
21 changes: 15 additions & 6 deletions src/Connection/StreamConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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
Expand All @@ -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);
}
}
6 changes: 6 additions & 0 deletions src/Exception/CommunicationFailed.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
2 changes: 1 addition & 1 deletion tests/Integration/ClientMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
9 changes: 7 additions & 2 deletions tests/Integration/Connection/ConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
}
Expand Down
9 changes: 7 additions & 2 deletions tests/Integration/Connection/ParseGreetingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,22 @@ 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();

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) {
Expand Down
27 changes: 21 additions & 6 deletions tests/Integration/Connection/ReadTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,34 +35,44 @@ 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();
}

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();
}
Expand Down Expand Up @@ -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();
}
Expand Down

0 comments on commit 8c99575

Please sign in to comment.