From 25e66bc9f05a0790a789beb49677181f606b43eb Mon Sep 17 00:00:00 2001 From: Adam Bodnar Date: Thu, 5 Mar 2026 11:56:20 -0600 Subject: [PATCH 1/3] fix: respect custom query timeout in ping() method ping() was only setting CURLOPT_CONNECTTIMEOUT via connectTimeOut() but never called timeOut(), so CURLOPT_TIMEOUT always fell back to the hard-coded CurlerRequest default of 10 seconds, ignoring any value set via setTimeout(). Add .timeOut($this->settings()->getTimeOut()) to the request chain, matching the pattern already used in newRequest(). Co-Authored-By: Claude --- src/Transport/Http.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Transport/Http.php b/src/Transport/Http.php index 9e9618d..5389e20 100644 --- a/src/Transport/Http.php +++ b/src/Transport/Http.php @@ -577,7 +577,7 @@ public function getRequestWrite(Query $query): CurlerRequest public function ping(): bool { $request = new CurlerRequest(); - $request->url($this->getUri())->verbose(false)->GET()->connectTimeOut($this->getConnectTimeOut()); + $request->url($this->getUri())->verbose(false)->GET()->timeOut($this->settings()->getTimeOut())->connectTimeOut($this->getConnectTimeOut()); $this->_curler->execOne($request); return trim($request->response()->body()) === 'Ok.'; From e59d3114c3b570cc1150d35956862e2ae11abced Mon Sep 17 00:00:00 2001 From: Adam Bodnar Date: Thu, 5 Mar 2026 12:03:15 -0600 Subject: [PATCH 2/3] test: add test verifying setTimeout() is respected by ping() Uses a local TCP server that completes the handshake but never sends an HTTP response, isolating CURLOPT_TIMEOUT from CURLOPT_CONNECTTIMEOUT so the assertion targets exactly the bug fixed in ping(). Co-Authored-By: Claude --- tests/ClientTest.php | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 13f7f50..571bef0 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -423,6 +423,50 @@ public function testConnectTimeout() $this->assertEquals(1, $use_time); } + /** + * Verifies that setTimeout() is respected by ping(). + * + * A local TCP server that accepts connections but never sends an HTTP + * response is used so that the TCP handshake succeeds (preventing the + * connect timeout from firing first) while the overall request hangs + * until CURLOPT_TIMEOUT fires. + */ + public function testQueryTimeoutIsRespectedByPing(): void + { + // Bind to an ephemeral port; the OS completes the TCP handshake for + // queued connections, but we never call stream_socket_accept(), so + // the HTTP response never arrives. + $server = stream_socket_server('tcp://127.0.0.1:0', $errno, $errstr); + $this->assertNotFalse($server, "Could not start local test server: $errstr"); + + $address = stream_socket_get_name($server, false); + [$host, $port] = explode(':', $address); + + $config = [ + 'host' => $host, + 'port' => (int) $port, + 'username' => '', + 'password' => '', + ]; + + $start_time = microtime(true); + + try { + $db = new Client($config); + // High connect timeout so it does not fire before the query timeout. + $db->setConnectTimeOut(5); + $db->setTimeout(1); + $db->ping(); + } catch (\Exception $e) { + // Expected — ping() will throw when the query timeout fires. + } finally { + fclose($server); + } + + $elapsed = round(microtime(true) - $start_time); + $this->assertEquals(1, $elapsed); + } + /** * */ From f0f159c48beb7f627f173cfb7eccc65de1c2eb5e Mon Sep 17 00:00:00 2001 From: Adam Bodnar Date: Thu, 5 Mar 2026 12:07:34 -0600 Subject: [PATCH 3/3] test: move ping timeout test to standalone PingTimeoutTest class ClientTest.setUp() requires a live ClickHouse connection, so the timeout test (which only needs a local socket server) is extracted into its own class to run without any external dependencies. Co-Authored-By: Claude --- tests/ClientTest.php | 44 ---------------------------- tests/PingTimeoutTest.php | 61 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 44 deletions(-) create mode 100644 tests/PingTimeoutTest.php diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 571bef0..13f7f50 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -423,50 +423,6 @@ public function testConnectTimeout() $this->assertEquals(1, $use_time); } - /** - * Verifies that setTimeout() is respected by ping(). - * - * A local TCP server that accepts connections but never sends an HTTP - * response is used so that the TCP handshake succeeds (preventing the - * connect timeout from firing first) while the overall request hangs - * until CURLOPT_TIMEOUT fires. - */ - public function testQueryTimeoutIsRespectedByPing(): void - { - // Bind to an ephemeral port; the OS completes the TCP handshake for - // queued connections, but we never call stream_socket_accept(), so - // the HTTP response never arrives. - $server = stream_socket_server('tcp://127.0.0.1:0', $errno, $errstr); - $this->assertNotFalse($server, "Could not start local test server: $errstr"); - - $address = stream_socket_get_name($server, false); - [$host, $port] = explode(':', $address); - - $config = [ - 'host' => $host, - 'port' => (int) $port, - 'username' => '', - 'password' => '', - ]; - - $start_time = microtime(true); - - try { - $db = new Client($config); - // High connect timeout so it does not fire before the query timeout. - $db->setConnectTimeOut(5); - $db->setTimeout(1); - $db->ping(); - } catch (\Exception $e) { - // Expected — ping() will throw when the query timeout fires. - } finally { - fclose($server); - } - - $elapsed = round(microtime(true) - $start_time); - $this->assertEquals(1, $elapsed); - } - /** * */ diff --git a/tests/PingTimeoutTest.php b/tests/PingTimeoutTest.php new file mode 100644 index 0000000..25afb62 --- /dev/null +++ b/tests/PingTimeoutTest.php @@ -0,0 +1,61 @@ +assertNotFalse($server, "Could not start local test server: $errstr"); + + $address = stream_socket_get_name($server, false); + [$host, $port] = explode(':', $address); + + $config = [ + 'host' => $host, + 'port' => (int) $port, + 'username' => '', + 'password' => '', + ]; + + $start_time = microtime(true); + + try { + $db = new Client($config); + // High connect timeout so it does not fire before the query timeout. + $db->setConnectTimeOut(5); + $db->setTimeout(1); + $db->ping(); + } catch (\Exception $e) { + // Expected — ping() will throw when the query timeout fires. + } finally { + fclose($server); + } + + $elapsed = round(microtime(true) - $start_time); + $this->assertEquals(1, $elapsed); + } +}