From 27481150b5d1cff6ed657b1df7553a8c172fb71e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 2 May 2025 19:58:54 +0200 Subject: [PATCH 1/2] Improve error messages for invalid responses --- src/EventSource.php | 21 +++++++++++-- tests/EventSourceTest.php | 62 +++++++++++++++++++++++++++++++++++---- 2 files changed, 74 insertions(+), 9 deletions(-) diff --git a/src/EventSource.php b/src/EventSource.php index 8e29182..f34f62b 100644 --- a/src/EventSource.php +++ b/src/EventSource.php @@ -193,15 +193,20 @@ private function request() $this->request->then(function (ResponseInterface $response) { if ($response->getStatusCode() !== 200) { $this->readyState = self::CLOSED; - $this->emit('error', array(new \UnexpectedValueException('Unexpected status code'))); + $this->emit('error', [new \UnexpectedValueException( + 'Expected "200 OK" response status, ' . $this->quote($response->getStatusCode() . ' ' . $response->getReasonPhrase()) . ' response status returned' + )]); $this->close(); return; } // match `Content-Type: text/event-stream` (case insensitive and ignore additional parameters) - if (!preg_match('/^text\/event-stream(?:$|;)/i', $response->getHeaderLine('Content-Type'))) { + $contentType = $response->getHeaderLine('Content-Type'); + if (!preg_match('/^text\/event-stream(?:$|;)/i', $contentType)) { $this->readyState = self::CLOSED; - $this->emit('error', array(new \UnexpectedValueException('Unexpected Content-Type'))); + $this->emit('error', [new \UnexpectedValueException( + 'Expected "Content-Type: text/event-stream" response header, ' . (!$response->hasHeader('Content-Type') ? 'no "Content-Type"' : $this->quote('Content-Type: ' . $contentType)) . ' response header returned' + )]); $this->close(); return; } @@ -290,4 +295,14 @@ public function close() $this->removeAllListeners(); } + + /** + * @param string $string + * @return string + * @throws void + */ + private function quote($string) + { + return '"' . \addcslashes(\substr($string, 0, 100), "\x00..\x1f\"\\\x7f..\xff") . '"'; + } } diff --git a/tests/EventSourceTest.php b/tests/EventSourceTest.php index 1b58680..57db204 100644 --- a/tests/EventSourceTest.php +++ b/tests/EventSourceTest.php @@ -228,7 +228,28 @@ public function testCloseAfterGetRequestFromConstructorFailsWillCancelPendingRet $es->close(); } - public function testConstructorWillReportFatalErrorWhenGetResponseResolvesWithInvalidStatusCode() + public function provideInvalidStatusCode() + { + return [ + [ + new Response(400, ['Content-Type' => 'text/event-stream'], ''), + 'Expected "200 OK" response status, "400 Bad Request" response status returned' + ], + [ + new Response(500, ['Content-Type' => 'text/event-stream'], '', '1.1', "Intern\xE4l Server Err\xF6r"), + 'Expected "200 OK" response status, "500 Intern\344l Server Err\366r" response status returned' + ], + [ + new Response(400, ['Content-Type' => 'text/event-stream'], '', '1.1', str_repeat('a', 200)), + 'Expected "200 OK" response status, "400 ' . str_repeat('a', 96) . '" response status returned' + ] + ]; + } + + /** + * @dataProvider provideInvalidStatusCode + */ + public function testConstructorWillReportFatalErrorWhenGetResponseResolvesWithInvalidStatusCode($response, $expectedMessage) { $deferred = new Deferred(); $browser = $this->getMockBuilder('React\Http\Browser')->disableOriginalConstructor()->getMock(); @@ -244,14 +265,43 @@ public function testConstructorWillReportFatalErrorWhenGetResponseResolvesWithIn $caught = $e; }); - $response = new Response(400, array('Content-Type' => 'text/event-stream'), ''); $deferred->resolve($response); $this->assertEquals(EventSource::CLOSED, $readyState); $this->assertInstanceOf('UnexpectedValueException', $caught); - } - - public function testConstructorWillReportFatalErrorWhenGetResponseResolvesWithInvalidContentType() + $this->assertEquals($expectedMessage, $caught->getMessage()); + } + + public function provideInvalidContentType() + { + return [ + [ + new Response(200, [], ''), + 'Expected "Content-Type: text/event-stream" response header, no "Content-Type" response header returned' + ], + [ + new Response(200, ['Content-Type' => ''], ''), + 'Expected "Content-Type: text/event-stream" response header, "Content-Type: " response header returned' + ], + [ + new Response(200, ['Content-Type' => 'text/html'], ''), + 'Expected "Content-Type: text/event-stream" response header, "Content-Type: text/html" response header returned' + ], + [ + new Response(200, ['Content-Type' => "application/json; invalid=a\xE4b"], ''), + 'Expected "Content-Type: text/event-stream" response header, "Content-Type: application/json; invalid=a\344b" response header returned' + ], + [ + new Response(200, ['Content-Type' => str_repeat('a', 200)], ''), + 'Expected "Content-Type: text/event-stream" response header, "Content-Type: ' . str_repeat('a', 86) . '" response header returned' + ] + ]; + } + + /** + * @dataProvider provideInvalidContentType + */ + public function testConstructorWillReportFatalErrorWhenGetResponseResolvesWithInvalidContentType($response, $expectedMessage) { $deferred = new Deferred(); $browser = $this->getMockBuilder('React\Http\Browser')->disableOriginalConstructor()->getMock(); @@ -267,11 +317,11 @@ public function testConstructorWillReportFatalErrorWhenGetResponseResolvesWithIn $caught = $e; }); - $response = new Response(200, array(), ''); $deferred->resolve($response); $this->assertEquals(EventSource::CLOSED, $readyState); $this->assertInstanceOf('UnexpectedValueException', $caught); + $this->assertEquals($expectedMessage, $caught->getMessage()); } public function testConstructorWillReportOpenWhenGetResponseResolvesWithValidResponse() From dfe388d7beeb794da9576cda49adb7242f551bf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sun, 1 Mar 2026 22:08:29 +0100 Subject: [PATCH 2/2] Improve error reporting for invalid responses by using `ResponseException` --- README.md | 5 ++++- src/EventSource.php | 7 +++++-- tests/EventSourceTest.php | 7 +++++-- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 8c888ff..9a5c1e3 100644 --- a/README.md +++ b/README.md @@ -193,7 +193,7 @@ The `error` event will be emitted when the EventSource connection fails. The event receives a single `Exception` argument for the error instance. ```php -$redis->on('error', function (Exception $e) { +$es->on('error', function (Exception $e) { echo 'Error: ' . $e->getMessage() . PHP_EOL; }); ``` @@ -201,6 +201,9 @@ $redis->on('error', function (Exception $e) { The EventSource connection will be retried automatically when it is temporarily disconnected. If the server sends a non-successful HTTP status code or an invalid `Content-Type` response header, the connection will fail permanently. +In this case, the `error` event will receive a +[`ResponseException`](https://github.com/reactphp/http#responseexception) +that provides access to the response via the `getResponse()` method. ```php $es->on('error', function (Exception $e) use ($es) { diff --git a/src/EventSource.php b/src/EventSource.php index f34f62b..72cbe51 100644 --- a/src/EventSource.php +++ b/src/EventSource.php @@ -7,6 +7,7 @@ use React\EventLoop\Loop; use React\EventLoop\LoopInterface; use React\Http\Browser; +use React\Http\Message\ResponseException; use React\Stream\ReadableStreamInterface; /** @@ -193,7 +194,8 @@ private function request() $this->request->then(function (ResponseInterface $response) { if ($response->getStatusCode() !== 200) { $this->readyState = self::CLOSED; - $this->emit('error', [new \UnexpectedValueException( + $this->emit('error', [new ResponseException( + $response, 'Expected "200 OK" response status, ' . $this->quote($response->getStatusCode() . ' ' . $response->getReasonPhrase()) . ' response status returned' )]); $this->close(); @@ -204,7 +206,8 @@ private function request() $contentType = $response->getHeaderLine('Content-Type'); if (!preg_match('/^text\/event-stream(?:$|;)/i', $contentType)) { $this->readyState = self::CLOSED; - $this->emit('error', [new \UnexpectedValueException( + $this->emit('error', [new ResponseException( + $response, 'Expected "Content-Type: text/event-stream" response header, ' . (!$response->hasHeader('Content-Type') ? 'no "Content-Type"' : $this->quote('Content-Type: ' . $contentType)) . ' response header returned' )]); $this->close(); diff --git a/tests/EventSourceTest.php b/tests/EventSourceTest.php index 57db204..3f7dc6d 100644 --- a/tests/EventSourceTest.php +++ b/tests/EventSourceTest.php @@ -268,8 +268,10 @@ public function testConstructorWillReportFatalErrorWhenGetResponseResolvesWithIn $deferred->resolve($response); $this->assertEquals(EventSource::CLOSED, $readyState); - $this->assertInstanceOf('UnexpectedValueException', $caught); + $this->assertInstanceOf('React\Http\Message\ResponseException', $caught); $this->assertEquals($expectedMessage, $caught->getMessage()); + $this->assertEquals($response->getStatusCode(), $caught->getCode()); + $this->assertSame($response, $caught->getResponse()); } public function provideInvalidContentType() @@ -320,8 +322,9 @@ public function testConstructorWillReportFatalErrorWhenGetResponseResolvesWithIn $deferred->resolve($response); $this->assertEquals(EventSource::CLOSED, $readyState); - $this->assertInstanceOf('UnexpectedValueException', $caught); + $this->assertInstanceOf('React\Http\Message\ResponseException', $caught); $this->assertEquals($expectedMessage, $caught->getMessage()); + $this->assertSame($response, $caught->getResponse()); } public function testConstructorWillReportOpenWhenGetResponseResolvesWithValidResponse()