Merge pull request #205 from anathex/master

Fixed silent error occurrence on 200 http status code (ExceptionWhileProcessing query error)
smi2 · Jan 18, 2024 · a629f30 · a629f30
2 parents 975b6d6 + 1595cd0
commit a629f30
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 8 deletions.
diff --git a/src/Statement.php b/src/Statement.php
@@ -11,6 +11,8 @@
 
 class Statement implements \Iterator
 {
+    private const CLICKHOUSE_ERROR_REGEX = "%Code:\s(\d+)\.\s*DB::Exception\s*:\s*(.*)(?:,\s*e\.what|\(version).*%ius";
+
     /**
      * @var string|mixed
      */
@@ -133,23 +135,28 @@ public function sql()
      * @param string $body
      * @return array|bool
      */
-    private function parseErrorClickHouse($body)
+    private function parseErrorClickHouse(string $body)
     {
         $body = trim($body);
-        $mathes = [];
+        $matches = [];
 
         // Code: 115. DB::Exception: Unknown setting readonly[0], e.what() = DB::Exception
         // Code: 192. DB::Exception: Unknown user x, e.what() = DB::Exception
         // Code: 60. DB::Exception: Table default.ZZZZZ doesn't exist., e.what() = DB::Exception
         // Code: 516. DB::Exception: test_username: Authentication failed: password is incorrect or there is no user with such name. (AUTHENTICATION_FAILED) (version 22.8.3.13 (official build))
 
-        if (preg_match("%Code:\s(\d+).\s*DB\:\:Exception\s*:\s*(.*)(?:\,\s*e\.what|\(version).*%ius", $body, $mathes)) {
-            return ['code' => $mathes[1], 'message' => $mathes[2]];
+        if (preg_match(self::CLICKHOUSE_ERROR_REGEX, $body, $matches)) {
+            return ['code' => $matches[1], 'message' => $matches[2]];
         }
 
         return false;
     }
 
+    private function hasErrorClickhouse(string $body): bool {
+
+        return preg_match(self::CLICKHOUSE_ERROR_REGEX, $body) === 1;
+    }
+
     /**
      * @return bool
      * @throws Exception\TransportException
@@ -197,12 +204,24 @@ public function error()
      * @return bool
      * @throws Exception\TransportException
      */
-    public function isError()
+    public function isError(): bool
     {
-        return ($this->response()->http_code() !== 200 || $this->response()->error_no());
+        if ($this->response()->http_code() !== 200) {
+            return true;
+        }
+
+        if ($this->response()->error_no()) {
+            return true;
+        }
+
+        if ($this->hasErrorClickhouse($this->response()->body())) {
+            return true;
+        }
+
+        return false;
     }
 
-    private function check() : bool
+    private function check(): bool
     {
         if (!$this->_request->isResponseExists()) {
             throw QueryException::noResponse();

diff --git a/tests/StatementTest.php b/tests/StatementTest.php
@@ -17,6 +17,39 @@
  */
 final class StatementTest extends TestCase
 {
+    use WithClient;
+
+    public function testIsError()
+    {
+        $result = $this->client->select(
+            'SELECT throwIf(1=1, \'Raised error\');'
+        );
+
+        $this->assertGreaterThanOrEqual(500, $result->getRequest()->response()->http_code());
+        $this->assertTrue($result->isError());
+    }
+
+    /**
+     * @link https://github.com/smi2/phpClickHouse/issues/144
+     * @link https://clickhouse.com/docs/en/interfaces/http#http_response_codes_caveats
+     *
+     * During execution of query it is possible to get ExceptionWhileProcessing in Clickhouse
+     * In that case HTTP status code of Clickhouse interface would be 200
+     * and it is kind of "expected" behaviour of CH
+     */
+    public function testIsErrorWithOkStatusCode()
+    {
+        // value of "number" in query must be greater than 100 thousand
+        // for part of CH response to be flushed to client with 200 status code
+        // and further ExceptionWhileProcessing occurrence
+        $result = $this->client->select(
+            'SELECT number, throwIf(number=100100, \'Raised error\') FROM system.numbers;'
+        );
+
+        $this->assertEquals(200, $result->getRequest()->response()->http_code());
+        $this->assertTrue($result->isError());
+    }
+
     /**
      * @dataProvider dataProvider
      */
@@ -38,7 +71,7 @@ public function testParseErrorClickHouse(
         $this->assertInstanceOf(Statement::class, $statement);
 
         $this->expectException(DatabaseException::class);
-        $this->expectDeprecationMessage($exceptionMessage);
+        $this->expectExceptionMessage($exceptionMessage);
         $this->expectExceptionCode($exceptionCode);
 
         $statement->error();