From f77f32e12adb3a2abf3c52c4c846eec72efd1098 Mon Sep 17 00:00:00 2001 From: nick Date: Mon, 14 Sep 2026 20:26:29 +0700 Subject: [PATCH] Update retry handling for 503/504 errors, update tests --- docs/configuration.md | 4 +- src/Manticoresearch/Client.php | 19 ++-- .../Exceptions/ConnectionException.php | 6 +- .../Exceptions/ResponseException.php | 8 +- src/Manticoresearch/Response.php | 7 ++ test/Manticoresearch/ClientTest.php | 86 +++++++++++++++++++ test/Manticoresearch/ResponseTest.php | 17 +++- 7 files changed, 136 insertions(+), 11 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 75e5653..555ca62 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -194,6 +194,8 @@ $params = [ ### Retries +HTTP 503 and 504 responses are retried within the configured attempt limit, including empty bodies, HTML, and JSON without an `error` field. Their exception messages include the HTTP status. When attempts are exhausted, `NoMoreNodesException::getCode()` contains the last failure's HTTP status (or `0` for a connection failure), and `getPrevious()` provides the original exception. For a `ResponseException`, the status is also available through `getResponse()->getStatusCode()`. + By default, the number of retries is equal to the number of defined hosts. If the number of hosts is 10 and retries are set to 5, the query will retry on 5 hosts according to the connection strategy and end with an error after 5 attempts. @@ -218,4 +220,4 @@ A single host example: $params = ['host' => '123.0.0.1', 'port' => '1234', 'retries' => 2]; $client = new Client($params); ``` - \ No newline at end of file + diff --git a/src/Manticoresearch/Client.php b/src/Manticoresearch/Client.php index 85e20ab..f0301bb 100755 --- a/src/Manticoresearch/Client.php +++ b/src/Manticoresearch/Client.php @@ -443,16 +443,16 @@ public function request(Request $request, array $params = [], string $retryReaso $this->initConnections(); throw $e; } catch (ConnectionException | ResponseException $e) { + $e->setRequest($request); if (!$this->connectionPool->retries) { - throw new NoMoreNodesException($e); + throw new NoMoreNodesException($e->getMessage(), $request, $e->getCode(), $e); } if ($e instanceof ResponseException) { - // We apply retrying to 502-600 responses only + // Retry HTTP 503 and 504 responses only. if (!$e->getResponse()->is5xxRetriedStatus()) { throw $e; } - $e->setRequest($request); $failContext = 'bad response'; } else { $failContext = 'attempt'; @@ -460,7 +460,9 @@ public function request(Request $request, array $params = [], string $retryReaso $exMsg = $e->getMessage(); // We rely on the common error message format from Manticore here $exReasonPos = strrpos($exMsg, ':'); - $exceptionReason = substr($exMsg, ($exReasonPos === false) ? 0 : $exReasonPos + 1); + $exceptionReason = $e instanceof ResponseException + ? $exMsg + : substr($exMsg, ($exReasonPos === false) ? 0 : $exReasonPos + 1); $this->logger->warning( "Manticore Search Request failed on $failContext " . $this->connectionPool->retriesAttempts . ':', [ @@ -473,7 +475,14 @@ public function request(Request $request, array $params = [], string $retryReaso $connection->mark(false); } - return $this->request($request, $params, $exceptionReason); + try { + return $this->request($request, $params, $exceptionReason); + } catch (NoMoreNodesException $ex) { + if ($ex->getPrevious() !== null) { + throw $ex; + } + throw new NoMoreNodesException($ex->getMessage(), $request, $e->getCode(), $e); + } } return $this->lastResponse; diff --git a/src/Manticoresearch/Exceptions/ConnectionException.php b/src/Manticoresearch/Exceptions/ConnectionException.php index 1495e8f..12ee6f2 100644 --- a/src/Manticoresearch/Exceptions/ConnectionException.php +++ b/src/Manticoresearch/Exceptions/ConnectionException.php @@ -24,10 +24,12 @@ class ConnectionException extends \RuntimeException implements ExceptionInterfac * ConnectionException constructor. * @param string $message * @param Request|null $request + * @param int $code + * @param \Throwable|null $previous */ - public function __construct($message = '', ?Request $request = null) { + public function __construct($message = '', ?Request $request = null, int $code = 0, ?\Throwable $previous = null) { $this->request = $request; - parent::__construct($message); + parent::__construct($message, $code, $previous); } /** diff --git a/src/Manticoresearch/Exceptions/ResponseException.php b/src/Manticoresearch/Exceptions/ResponseException.php index 860566a..8f743be 100644 --- a/src/Manticoresearch/Exceptions/ResponseException.php +++ b/src/Manticoresearch/Exceptions/ResponseException.php @@ -34,7 +34,13 @@ public function __construct(Request $request, Response $response) { $this->request = $request; $this->response = $response; - parent::__construct($response->getError()); + $message = $response->getError(); + $code = 0; + if ($response->is5xxRetriedStatus()) { + $code = $response->getStatusCode(); + $message = 'HTTP ' . $code . ($message !== '' ? ': ' . $message : ''); + } + parent::__construct($message, $code); } /** diff --git a/src/Manticoresearch/Response.php b/src/Manticoresearch/Response.php index c0c0a6a..0ad0ea0 100755 --- a/src/Manticoresearch/Response.php +++ b/src/Manticoresearch/Response.php @@ -129,6 +129,9 @@ private function stripBadUtf8() { */ public function hasError() { $response = $this->getResponse(); + if ($this->is5xxRetriedStatus()) { + return true; + } if (is_array($response)) { foreach ($response as $r) { if (isset($r['error']) && $r['error'] !== '') { @@ -197,6 +200,10 @@ public function is5xxRetriedStatus() { return $this->status >= 503 && $this->status < 505; } + public function getStatusCode(): ?int { + return $this->status; + } + /** * set request info * @param array $info diff --git a/test/Manticoresearch/ClientTest.php b/test/Manticoresearch/ClientTest.php index 1cf5279..2e11b20 100644 --- a/test/Manticoresearch/ClientTest.php +++ b/test/Manticoresearch/ClientTest.php @@ -12,11 +12,14 @@ use Manticoresearch\Connection; use Manticoresearch\Connection\Strategy\Random; use Manticoresearch\Exceptions\ConnectionException; +use Manticoresearch\Exceptions\NoMoreNodesException; +use Manticoresearch\Exceptions\ResponseException; use Manticoresearch\Request; use Manticoresearch\Response; use Manticoresearch\Response\Token; use Manticoresearch\Table; use Manticoresearch\Test\Helper\PopulateHelperTest; +use Manticoresearch\Transport\TransportInterface; use PHPUnit\Framework\TestCase; class ClientTest extends TestCase @@ -213,6 +216,89 @@ public function testTokenEndpointCanReturnResponseObject() { $this->assertInstanceOf(Token::class, $client->token(true)); } + public function testRetryableResponsesRecover() { + foreach ([503, 504] as $status) { + foreach (['timeout', '', '{}', '{"message":"timeout"}', '{"error":"timeout"}'] as $body) { + $client = $this->createRetryClient( + [new Response($body, $status), new Response('{"ok":true}', 200)], + 2 + ); + $this->assertSame(['ok' => true], $client->request($this->createRetryRequest())->getResponse()); + $this->assertSame(0, $client->getConnectionPool()->retriesAttempts); + } + } + } + + public function testExhaustedRetriesPreserveStatusesAndLastException() { + $client = $this->createRetryClient( + [ + new Response('unavailable', 503), + new Response('{"error":"proxy: upstream timeout"}', 504), + ], + 2 + ); + $request = $this->createRetryRequest(); + try { + $client->request($request); + $this->fail('Expected exhausted retries'); + } catch (NoMoreNodesException $e) { + $this->assertStringContainsString('HTTP 503: Syntax error', $e->getMessage()); + $this->assertStringContainsString('HTTP 504: "proxy: upstream timeout"', $e->getMessage()); + $this->assertSame(504, $e->getCode()); + $this->assertSame($request, $e->getRequest()); + $this->assertInstanceOf(ResponseException::class, $e->getPrevious()); + $this->assertSame(504, $e->getPrevious()->getResponse()->getStatusCode()); + $this->assertSame($request, $e->getPrevious()->getRequest()); + } + } + + public function testDisabledRetriesPreserveHttpStatus() { + foreach ([503, 504] as $status) { + $client = $this->createRetryClient([new Response('{}', $status)], 0); + try { + $client->request($this->createRetryRequest()); + $this->fail('Expected HTTP failure'); + } catch (NoMoreNodesException $e) { + $this->assertSame('HTTP ' . $status, $e->getMessage()); + $this->assertSame($status, $e->getCode()); + $this->assertInstanceOf(ResponseException::class, $e->getPrevious()); + } + } + } + + public function testOtherHttpErrorsAreNotRetried() { + $client = $this->createRetryClient([new Response('{"error":"invalid query"}', 400)], 2); + $this->expectException(ResponseException::class); + $this->expectExceptionMessage('"invalid query"'); + $client->request($this->createRetryRequest()); + } + + private function createRetryClient(array $responses, int $retries): Client { + $transport = $this->createMock(TransportInterface::class); + $transport->expects($this->exactly(sizeof($responses)))->method('execute')->willReturnCallback( + static function (Request $request) use (&$responses) { + $response = array_shift($responses); + if ($response->hasError()) { + throw new ResponseException($request, $response); + } + return $response; + } + ); + $connection = $this->getMockBuilder(Connection::class) + ->setConstructorArgs([['persistent' => false]]) + ->onlyMethods(['getTransportHandler']) + ->getMock(); + $connection->expects($this->exactly(sizeof($responses)))->method('getTransportHandler')->willReturn($transport); + return new Client(['connections' => [$connection], 'retries' => $retries]); + } + + private function createRetryRequest(): Request { + $request = new Request(['body' => []]); + $request->setPath('/search'); + $request->setMethod('GET'); + return $request; + } + private function createTokenClient() { return new class extends Client { private $tokenRequest; diff --git a/test/Manticoresearch/ResponseTest.php b/test/Manticoresearch/ResponseTest.php index 0316e8e..a754fa5 100755 --- a/test/Manticoresearch/ResponseTest.php +++ b/test/Manticoresearch/ResponseTest.php @@ -47,7 +47,8 @@ public function test5xxInvalidJsonResponseException() { $response = new Response($payload, 503); $this->expectException(ResponseException::class); - $this->expectExceptionMessage('Syntax error'); + $this->expectExceptionMessage('HTTP 503: Syntax error'); + $this->expectExceptionCode(503); $response->getResponse(); } @@ -66,7 +67,19 @@ public function test504InvalidJsonWithBigintConversion() { $response->enableBigintConversion(); $this->expectException(ResponseException::class); - $this->expectExceptionMessage('Syntax error'); + $this->expectExceptionMessage('HTTP 504: Syntax error'); + $this->expectExceptionCode(504); $response->getResponse(); } + + public function testRetryableStatusWithValidJson() { + foreach ([503, 504] as $status) { + foreach (['{}', '{"message":"upstream timeout"}', 'null', '"timeout"'] as $body) { + $response = new Response($body, $status); + $this->assertTrue($response->hasError()); + $this->assertSame($status, $response->getStatusCode()); + } + } + $this->assertFalse((new Response('{"message":"ok"}', 200))->hasError()); + } }