From 2c8a9cac1bcee0e57d81669ede815fbd1541594c Mon Sep 17 00:00:00 2001 From: cdosoftei Date: Tue, 8 Feb 2022 17:49:00 -0500 Subject: [PATCH 1/2] :recycle: Improved exception throwing/handling --- src/InboundClient.php | 36 ++++++++++++++--------- src/InboundServer.php | 9 +++++- src/OutboundClient.php | 10 +++++-- src/OutboundServer.php | 9 +++++- tests/InboundClientTest.php | 55 ++++++++++++++++++++++-------------- tests/InboundServerTest.php | 15 ++++------ tests/OutboundClientTest.php | 6 ++++ tests/OutboundServerTest.php | 9 ++++-- 8 files changed, 98 insertions(+), 51 deletions(-) diff --git a/src/InboundClient.php b/src/InboundClient.php index a3d37b2..a680365 100644 --- a/src/InboundClient.php +++ b/src/InboundClient.php @@ -25,7 +25,7 @@ Response, ResponseInterface }; -USE Closure; +use Closure; use Throwable; /** @@ -105,12 +105,18 @@ public function connect(): PromiseInterface * Processes raw inbound bytes * * @param string $chunk - * @throws ReactESLException */ protected function dataHandler(string $chunk): void { $responses = []; - $ret = $this->esl->consume($chunk, $responses); + + try { + $ret = $this->esl->consume($chunk, $responses); + } catch (Throwable $t) { + $this->emit('error', [$t]); + + return; + } assert(!is_null($responses)); @@ -143,17 +149,21 @@ protected function dataHandler(string $chunk): void continue; } - if (empty($this->queue)) { + $deferred = array_shift($this->queue); + + if (is_null($deferred)) { $contentType = $response->getHeader(AbstractHeader::CONTENT_TYPE) ?? ''; - throw new ReactESLException( - 'Unexpected reply received (ENOMSG) ' . $contentType, - defined('SOCKET_ENOMSG') ? SOCKET_ENOMSG : 42 + $this->emit( + 'error', + [new ReactESLException( + 'Unexpected reply received (ENOMSG) ' . $contentType, + defined('SOCKET_ENOMSG') ? SOCKET_ENOMSG : 42 + )] ); + } else { + $deferred->resolve($response); } - - $deferred = array_shift($this->queue); - $deferred->resolve($response); } } @@ -161,8 +171,6 @@ protected function dataHandler(string $chunk): void * Processes early inbound messages (authentication) * * @param MessageInterface $response - * - * @throws ReactESLException */ public function preAuthHandler(MessageInterface $response): void { @@ -181,14 +189,14 @@ public function preAuthHandler(MessageInterface $response): void $this->authenticated = true; $this->deferredConnect->resolve($this); } else { - throw new ReactESLException('Authentication failed'); + $this->deferredConnect->reject(new ReactESLException('Authentication failed')); } } else if ($response instanceof Response\TextDisconnectNotice) { $this->emit('disconnect', [$response]); } else if ($response instanceof Response\TextRudeRejection) { $this->deferredConnect->reject(new ReactESLException('Access denied')); } else { - throw new ReactESLException('Unexpected response (expecting auth/request)'); + $this->deferredConnect->reject(new ReactESLException('Unexpected response (expecting auth/request)')); } } diff --git a/src/InboundServer.php b/src/InboundServer.php index 91027bd..ce60b5f 100644 --- a/src/InboundServer.php +++ b/src/InboundServer.php @@ -17,6 +17,7 @@ Request }; use Closure; +use Throwable; /** * Inbound ESL Server class @@ -69,7 +70,13 @@ protected function connectionHandler(DuplexStreamInterface $stream): void $client = new RemoteInboundClient($esl); $stream->on('data', function (string $chunk) use ($client, $esl) { - $esl->consume($chunk, $requests); + try { + $esl->consume($chunk, $requests); + } catch (Throwable $t) { + $client->emit('error', [$t]); + + return; + } assert(!is_null($requests)); diff --git a/src/OutboundClient.php b/src/OutboundClient.php index 5c9aa9d..3c92ed5 100644 --- a/src/OutboundClient.php +++ b/src/OutboundClient.php @@ -83,12 +83,16 @@ public function connect(): PromiseInterface * Processes raw inbound bytes * * @param string $chunk - * @throws ReactESLException */ protected function dataHandler(string $chunk): void { - $requests = []; - $ret = $this->esl->consume($chunk, $requests); + try { + $this->esl->consume($chunk, $requests); + } catch (Throwable $t) { + $this->emit('error', [$t]); + + return; + } assert(!is_null($requests)); diff --git a/src/OutboundServer.php b/src/OutboundServer.php index 5ac871c..bfad5e4 100644 --- a/src/OutboundServer.php +++ b/src/OutboundServer.php @@ -20,6 +20,7 @@ Response }; use Closure; +use Throwable; /** * Outbound ESL Server class @@ -74,7 +75,13 @@ protected function connectionHandler(DuplexStreamInterface $stream): void $client = new RemoteOutboundClient($esl); $stream->on('data', function (string $chunk) use ($client, $esl) { - $esl->consume($chunk, $responses); + try { + $esl->consume($chunk, $responses); + } catch (Throwable $t) { + $client->emit('error', [$t]); + + return; + } assert(!is_null($responses)); diff --git a/tests/InboundClientTest.php b/tests/InboundClientTest.php index 6631c29..d56a4d2 100644 --- a/tests/InboundClientTest.php +++ b/tests/InboundClientTest.php @@ -114,6 +114,21 @@ public function testConnectFailed(): void $this->assertFalse($this->isPropertyInitialized($client, 'esl')); } + public function testDataHandlerOnParsingError(): void + { + $context = $this->prepareClientAndConnectors(); + $this->setPropertyValue($context->client, 'authenticated', true); + $dataHandler = $this->getMethod($context->client, 'dataHandler'); + + $this->setPropertyValue($context->client, 'esl', new AsyncConnection(AsyncConnection::INBOUND_CLIENT)); + + $context->client->on('error', function (\Throwable $t) { + $this->assertInstanceOf(ESL\Exception\ESLException::class, $t); + }); + + $dataHandler->invokeArgs($context->client, ["Content-Type: bogus/no-such-thing\n\n"]); + } + public function testDataHandlerOnPreAuth(): void { $context = $this->prepareClientAndConnectors(); @@ -253,6 +268,10 @@ public function testDataHandlerOnOutOfSequence(): void $response = "Content-Type: api/response\n\n"; + $context->client->on('error', function (\Throwable $t) { + $this->assertInstanceOf(ReactESLException::class, $t); + }); + $context->esl ->method('consume') ->with($response, []) @@ -262,7 +281,6 @@ public function testDataHandlerOnOutOfSequence(): void return ESL\Connection::SUCCESS; })); - $this->expectException(ReactESLException::class); $dataHandler->invokeArgs($context->client, [$response]); } @@ -334,17 +352,6 @@ public function testPreAuthHandlerOnCommandReplySuccess(): void $this->assertTrue($this->getPropertyValue($context->client, 'authenticated')); } - public function testPreAuthHandlerOnCommandReplyFailure(): void - { - $context = $this->prepareClientAndConnectors(); - $preAuthHandler = $this->getMethod($context->client, 'preAuthHandler'); - - $this->expectException(ReactESLException::class); - $preAuthHandler->invokeArgs($context->client, [ - (new ESL\Response\CommandReply)->setHeader(ESL\AbstractHeader::REPLY_TEXT, '-ERR invalid') - ]); - } - public function testPreAuthHandlerOnDisconnectNotice(): void { $context = $this->prepareClientAndConnectors(); @@ -377,8 +384,22 @@ public function testPreAuthHandlerOnUnexpectedResponse(): void $context = $this->prepareClientAndConnectors(); $preAuthHandler = $this->getMethod($context->client, 'preAuthHandler'); - $this->expectException(ReactESLException::class); $preAuthHandler->invokeArgs($context->client, [new ESL\Response\ApiResponse]); + $context->promise + ->then(function () { + $this->fail('Should never resolve'); + }) + ->otherwise(function (\Throwable $t) { + $this->assertInstanceOf(ReactESLException::class, $t); + }); + } + + public function testClose(): void + { + $context = $this->prepareClientAndConnectors(); + + $context->stream->expects($this->once())->method('close'); + $context->client->close(); } private function prepareClientAndConnectors(): stdClass @@ -408,12 +429,4 @@ private function prepareClientAndConnectors(): stdClass return $ret; } - - public function testClose(): void - { - $context = $this->prepareClientAndConnectors(); - - $context->stream->expects($this->once())->method('close'); - $context->client->close(); - } } diff --git a/tests/InboundServerTest.php b/tests/InboundServerTest.php index 2216dda..0fb59d1 100644 --- a/tests/InboundServerTest.php +++ b/tests/InboundServerTest.php @@ -85,25 +85,22 @@ public function testConnectionHandler(): void if ($response instanceof ESL\Response\AuthRequest) { return (new ESL\Request\Auth)->setParameters('ClueCon')->render(); } - - return ''; - } catch (ESL\Exception\ESLException $e) { - $request = ESL\Request::parse($bytes); - - $this->assertInstanceOf(ESL\Request\Api::class, $request); - $this->assertEquals('version', $request->getParameters()); - } + } catch (ESL\Exception\ESLException $e) {} return $bytes; }); $server->on('auth', function (RemoteInboundClient $client, ESL\Request\Auth $request) use ($stream) { + $client->on('error', function (\Throwable $t) { + $this->assertInstanceOf(ESL\Exception\ESLException::class, $t); + }); + $this->assertEquals('ClueCon', $request->getParameters()); - $client->send((new ESL\Response\CommandReply)->setHeader('reply-text', '+OK accepted')); $client->setAuthenticated(true); $stream->write("api version\n\n"); + $stream->write("bogus\n\n"); }); $connHandler->invokeArgs($server, [$stream]); diff --git a/tests/OutboundClientTest.php b/tests/OutboundClientTest.php index 41ae892..a56f7d0 100644 --- a/tests/OutboundClientTest.php +++ b/tests/OutboundClientTest.php @@ -119,6 +119,12 @@ public function testDataHandler(): void "linger\n\n" . "myevents json\n\n" ]); + + $client->on('error', function (\Throwable $t) { + $this->assertInstanceOf(ESL\Exception\ESLException::class, $t); + }); + + $dataHandler->invokeArgs($client, ["bogus\n\n"]); } public function testClose(): void diff --git a/tests/OutboundServerTest.php b/tests/OutboundServerTest.php index ccc29e1..709a2b8 100644 --- a/tests/OutboundServerTest.php +++ b/tests/OutboundServerTest.php @@ -129,13 +129,13 @@ public function testConnectionHandler(): void $client->on('error', function (\Throwable $t) { if ($t instanceof ReactESLException) { $this->assertTrue(true, 'Received out of sequence exception'); + } else if ($t instanceof ESL\Exception\ESLException) { + $this->assertTrue(true, 'Received parsing exception'); } else { $this->fail('Unrecognized exception'); } }); - $stream->write("content-type: api/response\nexpected: false\n\n"); - $deferred = new Deferred; $this->setPropertyValue($client, 'queue', [$deferred]); @@ -146,6 +146,11 @@ public function testConnectionHandler(): void $this->assertEquals('true', $response->getHeader('expected')); }); + $this->setPropertyValue($client, 'queue', []); + $stream->write("content-type: api/response\nexpected: false\n\n"); + + $stream->write("content-type: bogus\n\n"); + $stream->close(); }); From 8b70f5547b1325740cc2f64cc0bc948ffde15ed0 Mon Sep 17 00:00:00 2001 From: cdosoftei Date: Tue, 8 Feb 2022 17:52:35 -0500 Subject: [PATCH 2/2] :rocket: v0.8.5 --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index e9811e9..8036091 100644 --- a/composer.json +++ b/composer.json @@ -1,7 +1,7 @@ { "name": "rtckit/react-esl", "description": "Asynchronous FreeSWITCH Event Socket Layer (ESL) Library", - "version": "0.8.0", + "version": "0.8.5", "type": "library", "keywords": [ "freeswitch", @@ -34,7 +34,7 @@ "clue/stdio-react": "^2.5", "phpstan/phpstan": "^1.4", "phpunit/phpunit": "^9.5", - "vimeo/psalm": "^4.18" + "vimeo/psalm": "^4.20" }, "autoload": { "psr-4": {