diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b867102..e984947 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -59,7 +59,7 @@ jobs: - run: composer install --prefer-dist - - run: composer require php-http/discovery php-http/curl-client guzzlehttp/psr7 php-http/message php-http/message-factory http-interop/http-factory-guzzle + - run: composer require php-http/discovery php-http/curl-client 'guzzlehttp/psr7:^2.0' php-http/message php-http/message-factory http-interop/http-factory-guzzle - name: Bootstrap auth Manticore run: | @@ -102,7 +102,7 @@ jobs: extensions: curl - run: | composer install --prefer-dist - composer require php-http/discovery php-http/curl-client guzzlehttp/psr7 php-http/message http-interop/http-factory-guzzle + composer require php-http/discovery php-http/curl-client 'guzzlehttp/psr7:^2.0' php-http/message http-interop/http-factory-guzzle composer phpstan codestyle: diff --git a/composer.json b/composer.json index 4ff3032..95bd581 100644 --- a/composer.json +++ b/composer.json @@ -9,10 +9,14 @@ "license" : "MIT", "type" : "library", "require" : { - "php" : "^8.2", - "php-http/discovery" : "^1.14", - "ext-json" : "*", - "psr/log" : "^1.1 || ^2.0 || ^3.0" + "php": "^8.2", + "ext-json": "*", + "guzzlehttp/psr7": "^2.0", + "http-interop/http-factory-guzzle": "^1.2", + "php-http/curl-client": "^2.4", + "php-http/discovery": "^1.20", + "php-http/message": "^1.16", + "psr/log": "^1.1 || ^2.0 || ^3.0" }, "suggest" : { "ext-curl" : "*", diff --git a/phpstan.neon b/phpstan.neon index 7740754..5caf272 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -2,3 +2,5 @@ parameters: level: 3 paths: - src + bootstrapFiles: + - stubs/curl_handle_stub.php \ No newline at end of file diff --git a/src/Manticoresearch/Client.php b/src/Manticoresearch/Client.php index f23359c..85e20ab 100755 --- a/src/Manticoresearch/Client.php +++ b/src/Manticoresearch/Client.php @@ -16,6 +16,7 @@ use Manticoresearch\Endpoints\Pq; use Manticoresearch\Exceptions\ConnectionException; use Manticoresearch\Exceptions\NoMoreNodesException; +use Manticoresearch\Exceptions\ResponseException; use Manticoresearch\Exceptions\RuntimeException; use Manticoresearch\Response\SqlToArray; use Manticoresearch\Response\Token; @@ -441,16 +442,27 @@ public function request(Request $request, array $params = [], string $retryReaso $this->initConnections(); throw $e; - } catch (ConnectionException $e) { + } catch (ConnectionException | ResponseException $e) { if (!$this->connectionPool->retries) { throw new NoMoreNodesException($e); } + + if ($e instanceof ResponseException) { + // We apply retrying to 502-600 responses only + if (!$e->getResponse()->is5xxRetriedStatus()) { + throw $e; + } + $e->setRequest($request); + $failContext = 'bad response'; + } else { + $failContext = 'attempt'; + } $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); $this->logger->warning( - 'Manticore Search Request failed on attempt ' . $this->connectionPool->retriesAttempts . ':', + "Manticore Search Request failed on $failContext " . $this->connectionPool->retriesAttempts . ':', [ 'exception' => $exMsg, 'request' => $e->getRequest()->toArray(), diff --git a/src/Manticoresearch/Connection.php b/src/Manticoresearch/Connection.php old mode 100644 new mode 100755 index d8b548a..1fe9e91 --- a/src/Manticoresearch/Connection.php +++ b/src/Manticoresearch/Connection.php @@ -357,8 +357,7 @@ public function mark(bool $state) { } /** - * @return resource|null - * + * @return resource|\CurlHandle */ public function getCurl() { return $this->curl ?? curl_init(); diff --git a/src/Manticoresearch/Exceptions/ResponseException.php b/src/Manticoresearch/Exceptions/ResponseException.php index 94848d2..860566a 100644 --- a/src/Manticoresearch/Exceptions/ResponseException.php +++ b/src/Manticoresearch/Exceptions/ResponseException.php @@ -44,6 +44,14 @@ public function getRequest() :Request { return $this->request; } + /** + * + * @param Request $request + */ + public function setRequest(Request $request): void { + $this->request = $request; + } + /** * @return Response */ diff --git a/src/Manticoresearch/Response.php b/src/Manticoresearch/Response.php index e192c85..c0c0a6a 100755 --- a/src/Manticoresearch/Response.php +++ b/src/Manticoresearch/Response.php @@ -15,6 +15,7 @@ * @author Adrian Nuta * @link https://manticoresearch.com */ +use Manticoresearch\Exceptions\ResponseException; use Manticoresearch\Exceptions\RuntimeException; /** @@ -60,6 +61,12 @@ class Response */ protected $bigIntToString = false; + /** + * Response error message + * @var string + */ + protected $error = ''; + public function __construct($responseString, $status = null, $params = []) { if (is_array($responseString)) { $this->response = $responseString; @@ -75,24 +82,36 @@ public function __construct($responseString, $status = null, $params = []) { * @return array */ public function getResponse() { - if (null === $this->response) { - $this->response = $this->bigIntToString - ? json_decode($this->string, true, 512, JSON_BIGINT_AS_STRING) - : json_decode($this->string, true); - if (json_last_error() !== JSON_ERROR_NONE) { - if (json_last_error() !== JSON_ERROR_UTF8 || !$this->stripBadUtf8()) { - throw new RuntimeException( - 'fatal error while trying to decode JSON response: ' . json_last_error_msg() - ); - } + if (null !== $this->response) { + return $this->response; + } - $this->response = json_decode(preg_replace('/[\x00-\x1F\x80-\xFF]/', '', $this->string), true); + $this->response = $this->bigIntToString + ? json_decode($this->string, true, 512, JSON_BIGINT_AS_STRING) + : json_decode($this->string, true); + + if (json_last_error() !== JSON_ERROR_NONE) { + // If server returns 5xx error, we suppose it to be temporary and attempt retries + if ($this->is5xxRetriedStatus()) { + $this->error = json_last_error_msg(); + throw new ResponseException( + new Request(), + $this + ); } - - if (empty($this->response)) { - $this->response = []; + if (json_last_error() !== JSON_ERROR_UTF8 || !$this->stripBadUtf8()) { + throw new RuntimeException( + 'fatal error while trying to decode JSON response: ' . json_last_error_msg() + ); } + + $this->response = json_decode(preg_replace('/[\x00-\x1F\x80-\xFF]/', '', $this->string), true); } + + if (empty($this->response)) { + $this->response = []; + } + return $this->response; } @@ -126,6 +145,9 @@ public function hasError() { * @return false|string */ public function getError() { + if ($this->error) { + return $this->error; + } $response = $this->getResponse(); if (isset($response['error'])) { return json_encode($response['error'], true); @@ -168,6 +190,13 @@ public function getTime() { return $this->time; } + /* + * @return bool + */ + public function is5xxRetriedStatus() { + return $this->status >= 503 && $this->status < 505; + } + /** * set request info * @param array $info diff --git a/stubs/curl_handle_stub.php b/stubs/curl_handle_stub.php new file mode 100644 index 0000000..97d3245 --- /dev/null +++ b/stubs/curl_handle_stub.php @@ -0,0 +1,6 @@ +populateForKeywords(); static::$client = $helper->getClient(); diff --git a/test/Manticoresearch/ResponseTest.php b/test/Manticoresearch/ResponseTest.php old mode 100644 new mode 100755 index e26aad1..0316e8e --- a/test/Manticoresearch/ResponseTest.php +++ b/test/Manticoresearch/ResponseTest.php @@ -7,6 +7,7 @@ namespace Manticoresearch\Test; +use Manticoresearch\Exceptions\ResponseException; use Manticoresearch\Exceptions\RuntimeException; use Manticoresearch\Response; use PHPUnit\Framework\TestCase; @@ -40,4 +41,32 @@ public function testConstructorWithInvalidJSON() { $this->expectException(RuntimeException::class); $response->getResponse(); } + + public function test5xxInvalidJsonResponseException() { + $payload = '{invalid: json]'; + $response = new Response($payload, 503); + + $this->expectException(ResponseException::class); + $this->expectExceptionMessage('Syntax error'); + $response->getResponse(); + } + + public function testBigintConversion() { + $payload = '{"id":18446744073709551615}'; + $response = new Response($payload); + $response->enableBigintConversion(); + + $this->assertSame(['id' => '18446744073709551615'], $response->getResponse()); + $this->assertSame(['id' => '18446744073709551615'], $response->getResponse()); + $this->assertIsFloat((new Response($payload))->getResponse()['id']); + } + + public function test504InvalidJsonWithBigintConversion() { + $response = new Response('{invalid: json]', 504); + $response->enableBigintConversion(); + + $this->expectException(ResponseException::class); + $this->expectExceptionMessage('Syntax error'); + $response->getResponse(); + } }