feat: added 5xx bad json response handling

.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:

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" : "*",

phpstan.neon

@@ -2,3 +2,5 @@ parameters:
 	level: 3
 	paths:
 		- src
+	bootstrapFiles:
+		- stubs/curl_handle_stub.php

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(),

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();

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
 	 */

src/Manticoresearch/Response.php

@@ -15,6 +15,7 @@
  * @author Adrian Nuta <adrian.nuta@manticoresearch.com>
  * @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

stubs/curl_handle_stub.php

@@ -0,0 +1,6 @@
+<?php
+
+// Polyfill for PHP versions/environment where CurlHandle class is unavailable (e.g. PHP < 8.0)
+if (!class_exists('CurlHandle')) {
+	class CurlHandle {}
+}

test/Manticoresearch/Endpoints/KeywordsTest.php

@@ -19,7 +19,6 @@ class KeywordsTest extends \PHPUnit\Framework\TestCase
 
 	public static function setUpBeforeClass(): void {
 		parent::setUpBeforeClass();
-
 		$helper = new PopulateHelperTest('testDummy');
 		$helper->populateForKeywords();
 		static::$client = $helper->getClient();

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();
+	}
 }