Merge pull request #205 from anathex/master

Fixed silent error occurrence on 200 http status code (ExceptionWhileProcessing query error)

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

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