Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error reporting when custom error handler is used #290

Merged
merged 1 commit into from
Apr 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions src/FdServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,21 @@ public function __construct($fd, LoopInterface $loop = null)

$this->loop = $loop ?: Loop::get();

$this->master = @\fopen('php://fd/' . $fd, 'r+');
if (false === $this->master) {
$errno = 0;
$errstr = '';
\set_error_handler(function ($_, $error) use (&$errno, &$errstr) {
// Match errstr from PHP's warning message.
// fopen(php://fd/3): Failed to open stream: Error duping file descriptor 3; possibly it doesn't exist: [9]: Bad file descriptor
$error = \error_get_last();
\preg_match('/\[(\d+)\]: (.*)/', $error['message'], $m);
\preg_match('/\[(\d+)\]: (.*)/', $error, $m);
$errno = isset($m[1]) ? (int) $m[1] : 0;
$errstr = isset($m[2]) ? $m[2] : $error['message'];
$errstr = isset($m[2]) ? $m[2] : $error;
});

$this->master = \fopen('php://fd/' . $fd, 'r+');

\restore_error_handler();

if (false === $this->master) {
throw new \RuntimeException(
'Failed to listen on FD ' . $fd . ': ' . $errstr . SocketServer::errconst($errno),
$errno
Expand Down
17 changes: 11 additions & 6 deletions src/SocketServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,20 @@ public function close()
*/
public static function accept($socket)
{
$newSocket = @\stream_socket_accept($socket, 0);

if (false === $newSocket) {
$errno = 0;
$errstr = '';
\set_error_handler(function ($_, $error) use (&$errno, &$errstr) {
// Match errstr from PHP's warning message.
// stream_socket_accept(): accept failed: Connection timed out
$error = \error_get_last();
$errstr = \preg_replace('#.*: #', '', $error['message']);
$errno = self::errno($errstr);
$errstr = \preg_replace('#.*: #', '', $error);
$errno = SocketServer::errno($errstr);
});

$newSocket = \stream_socket_accept($socket, 0);

\restore_error_handler();

if (false === $newSocket) {
throw new \RuntimeException(
'Unable to accept new connection: ' . $errstr . self::errconst($errno),
$errno
Expand Down
20 changes: 13 additions & 7 deletions src/TcpConnector.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,19 @@ public function connect($uri)
// Linux reports socket errno and errstr again when trying to write to the dead socket.
// Suppress error reporting to get error message below and close dead socket before rejecting.
// This is only known to work on Linux, Mac and Windows are known to not support this.
@\fwrite($stream, \PHP_EOL);
$error = \error_get_last();

// fwrite(): send of 2 bytes failed with errno=111 Connection refused
\preg_match('/errno=(\d+) (.+)/', $error['message'], $m);
$errno = isset($m[1]) ? (int) $m[1] : 0;
$errstr = isset($m[2]) ? $m[2] : $error['message'];
$errno = 0;
$errstr = '';
\set_error_handler(function ($_, $error) use (&$errno, &$errstr) {
// Match errstr from PHP's warning message.
// fwrite(): send of 1 bytes failed with errno=111 Connection refused
\preg_match('/errno=(\d+) (.+)/', $error, $m);
$errno = isset($m[1]) ? (int) $m[1] : 0;
$errstr = isset($m[2]) ? $m[2] : $error;
});

\fwrite($stream, \PHP_EOL);

\restore_error_handler();
} else {
// Not on Linux and ext-sockets not available? Too bad.
$errno = \defined('SOCKET_ECONNREFUSED') ? \SOCKET_ECONNREFUSED : 111;
Expand Down
28 changes: 16 additions & 12 deletions src/UnixServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,25 +63,29 @@ public function __construct($path, LoopInterface $loop = null, array $context =
);
}

$this->master = @\stream_socket_server(
$errno = 0;
$errstr = '';
\set_error_handler(function ($_, $error) use (&$errno, &$errstr) {
// PHP does not seem to report errno/errstr for Unix domain sockets (UDS) right now.
// This only applies to UDS server sockets, see also https://3v4l.org/NAhpr.
// Parse PHP warning message containing unknown error, HHVM reports proper info at least.
if (\preg_match('/\(([^\)]+)\)|\[(\d+)\]: (.*)/', $error, $match)) {
$errstr = isset($match[3]) ? $match['3'] : $match[1];
$errno = isset($match[2]) ? (int)$match[2] : 0;
}
});

$this->master = \stream_socket_server(
$path,
$errno,
$errstr,
\STREAM_SERVER_BIND | \STREAM_SERVER_LISTEN,
\stream_context_create(array('socket' => $context))
);
if (false === $this->master) {
// PHP does not seem to report errno/errstr for Unix domain sockets (UDS) right now.
// This only applies to UDS server sockets, see also https://3v4l.org/NAhpr.
// Parse PHP warning message containing unknown error, HHVM reports proper info at least.
if ($errno === 0 && $errstr === '') {
$error = \error_get_last();
if (\preg_match('/\(([^\)]+)\)|\[(\d+)\]: (.*)/', $error['message'], $match)) {
$errstr = isset($match[3]) ? $match['3'] : $match[1];
$errno = isset($match[2]) ? (int)$match[2] : 0;
}
}

\restore_error_handler();

if (false === $this->master) {
throw new \RuntimeException(
'Failed to listen on Unix domain socket "' . $path . '": ' . $errstr . SocketServer::errconst($errno),
$errno
Expand Down
31 changes: 27 additions & 4 deletions tests/FdServerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function testCtorThrowsForInvalidUrl()
new FdServer('tcp://127.0.0.1:8080', $loop);
}

public function testCtorThrowsForUnknownFd()
public function testCtorThrowsForUnknownFdWithoutCallingCustomErrorHandler()
{
if (!is_dir('/dev/fd') || defined('HHVM_VERSION')) {
$this->markTestSkipped('Not supported on your platform');
Expand All @@ -62,12 +62,27 @@ public function testCtorThrowsForUnknownFd()
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
$loop->expects($this->never())->method('addReadStream');

$error = null;
set_error_handler(function ($_, $errstr) use (&$error) {
$error = $errstr;
});

$this->setExpectedException(
'RuntimeException',
'Failed to listen on FD ' . $fd . ': ' . (function_exists('socket_strerror') ? socket_strerror(SOCKET_EBADF) . ' (EBADF)' : 'Bad file descriptor'),
defined('SOCKET_EBADF') ? SOCKET_EBADF : 9
);
new FdServer($fd, $loop);

try {
new FdServer($fd, $loop);

restore_error_handler();
} catch (\Exception $e) {
restore_error_handler();
$this->assertNull($error);

throw $e;
}
}

public function testCtorThrowsIfFdIsAFileAndNotASocket()
Expand Down Expand Up @@ -319,7 +334,7 @@ public function testServerEmitsConnectionEventForNewConnection()
$server->close();
}

public function testEmitsErrorWhenAcceptListenerFails()
public function testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler()
{
if (!is_dir('/dev/fd') || defined('HHVM_VERSION')) {
$this->markTestSkipped('Not supported on your platform');
Expand All @@ -346,10 +361,18 @@ public function testEmitsErrorWhenAcceptListenerFails()
$this->assertNotNull($listener);
$socket = stream_socket_server('tcp://127.0.0.1:0');

$error = null;
set_error_handler(function ($_, $errstr) use (&$error) {
$error = $errstr;
});

$time = microtime(true);
$listener($socket);
$time = microtime(true) - $time;

restore_error_handler();
$this->assertNull($error);

$this->assertLessThan(1, $time);

$this->assertInstanceOf('RuntimeException', $exception);
Expand All @@ -362,7 +385,7 @@ public function testEmitsErrorWhenAcceptListenerFails()
/**
* @param \RuntimeException $e
* @requires extension sockets
* @depends testEmitsErrorWhenAcceptListenerFails
* @depends testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler
*/
public function testEmitsTimeoutErrorWhenAcceptListenerFails(\RuntimeException $exception)
{
Expand Down
19 changes: 17 additions & 2 deletions tests/TcpConnectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,32 @@ public function testConstructWithoutLoopAssignsLoopAutomatically()
}

/** @test */
public function connectionToEmptyPortShouldFail()
public function connectionToEmptyPortShouldFailWithoutCallingCustomErrorHandler()
{
$connector = new TcpConnector();
$promise = $connector->connect('127.0.0.1:9999');

$error = null;
set_error_handler(function ($_, $errstr) use (&$error) {
$error = $errstr;
});

$this->setExpectedException(
'RuntimeException',
'Connection to tcp://127.0.0.1:9999 failed: Connection refused' . (function_exists('socket_import_stream') ? ' (ECONNREFUSED)' : ''),
defined('SOCKET_ECONNREFUSED') ? SOCKET_ECONNREFUSED : 111
);
Block\await($promise, null, self::TIMEOUT);

try {
Block\await($promise, null, self::TIMEOUT);

restore_error_handler();
} catch (\Exception $e) {
restore_error_handler();
$this->assertNull($error);

throw $e;
}
}

/** @test */
Expand Down
12 changes: 10 additions & 2 deletions tests/TcpServerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ public function testCloseRemovesResourceFromLoop()
$server->close();
}

public function testEmitsErrorWhenAcceptListenerFails()
public function testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler()
{
$listener = null;
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
Expand All @@ -295,10 +295,18 @@ public function testEmitsErrorWhenAcceptListenerFails()
$this->assertNotNull($listener);
$socket = stream_socket_server('tcp://127.0.0.1:0');

$error = null;
set_error_handler(function ($_, $errstr) use (&$error) {
$error = $errstr;
});

$time = microtime(true);
$listener($socket);
$time = microtime(true) - $time;

restore_error_handler();
$this->assertNull($error);

$this->assertLessThan(1, $time);

$this->assertInstanceOf('RuntimeException', $exception);
Expand All @@ -311,7 +319,7 @@ public function testEmitsErrorWhenAcceptListenerFails()
/**
* @param \RuntimeException $e
* @requires extension sockets
* @depends testEmitsErrorWhenAcceptListenerFails
* @depends testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler
*/
public function testEmitsTimeoutErrorWhenAcceptListenerFails(\RuntimeException $exception)
{
Expand Down
31 changes: 27 additions & 4 deletions tests/UnixServerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,27 @@ public function testCtorThrowsForInvalidAddressScheme()
new UnixServer('tcp://localhost:0', $loop);
}

public function testCtorThrowsWhenPathIsNotWritable()
public function testCtorThrowsWhenPathIsNotWritableWithoutCallingCustomErrorHandler()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

$error = null;
set_error_handler(function ($_, $errstr) use (&$error) {
$error = $errstr;
});

$this->setExpectedException('RuntimeException');
$server = new UnixServer('/dev/null', $loop);

try {
new UnixServer('/dev/null', $loop);

restore_error_handler();
} catch (\Exception $e) {
restore_error_handler();
$this->assertNull($error);

throw $e;
}
}

public function testResumeWithoutPauseIsNoOp()
Expand Down Expand Up @@ -285,7 +300,7 @@ public function testCloseRemovesResourceFromLoop()
$server->close();
}

public function testEmitsErrorWhenAcceptListenerFails()
public function testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler()
{
$listener = null;
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
Expand All @@ -304,10 +319,18 @@ public function testEmitsErrorWhenAcceptListenerFails()
$this->assertNotNull($listener);
$socket = stream_socket_server('tcp://127.0.0.1:0');

$error = null;
set_error_handler(function ($_, $errstr) use (&$error) {
$error = $errstr;
});

$time = microtime(true);
$listener($socket);
$time = microtime(true) - $time;

restore_error_handler();
$this->assertNull($error);

$this->assertLessThan(1, $time);

$this->assertInstanceOf('RuntimeException', $exception);
Expand All @@ -320,7 +343,7 @@ public function testEmitsErrorWhenAcceptListenerFails()
/**
* @param \RuntimeException $e
* @requires extension sockets
* @depends testEmitsErrorWhenAcceptListenerFails
* @depends testEmitsErrorWhenAcceptListenerFailsWithoutCallingCustomErrorHandler
*/
public function testEmitsTimeoutErrorWhenAcceptListenerFails(\RuntimeException $exception)
{
Expand Down