From 3c66cbc10149b95d5a9048bb283992ce6042acfb Mon Sep 17 00:00:00 2001
From: Adam Rutkowski <hq@mtod.org>
Date: Mon, 18 Aug 2025 08:39:54 +0200
Subject: [PATCH] Raise an exception when reconnection fails

This hopefully prevents the situation in which
an unusable (e.g. closed) connection is returned to the pool,
causing the next client in queue fail to execute its query.

The expectation is, by propagating the `DBConnection.ConnectionError`
we'll make it caught by DBConnection and the broken connection will
be removed from the pool.
---
 lib/ch/connection.ex    |  4 +++-
 test/ch/faults_test.exs | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/lib/ch/connection.ex b/lib/ch/connection.ex
index f94b50a..931835c 100644
--- a/lib/ch/connection.ex
+++ b/lib/ch/connection.ex
@@ -420,7 +420,9 @@ defmodule Ch.Connection do
         # copy settings that are set dynamically (e.g. json as text) over to the new connection
         maybe_put_private(new_conn, :settings, HTTP.get_private(conn, :settings))
       else
-        _ -> conn
+        {:error, error} ->
+          Logger.warning("Could not reconnect due to #{inspect(error)}")
+          raise DBConnection.ConnectionError, "Reconnection failed: #{inspect(error)}"
       end
     end
   end
diff --git a/test/ch/faults_test.exs b/test/ch/faults_test.exs
index 9a41f4e..a91d1a0 100644
--- a/test/ch/faults_test.exs
+++ b/test/ch/faults_test.exs
@@ -573,6 +573,45 @@ defmodule Ch.FaultsTest do
                  " Expected \"#{expected_name}\" but got \"not-#{expected_name}\"!" <>
                  " Connection pooling might be unstable."
     end
+
+    test "raises exception when reconnection fails", ctx do
+      %{port: port, listen: listen, clickhouse: clickhouse} = ctx
+
+      {:ok, conn} = Ch.start_link(port: port)
+      {:ok, mint} = :gen_tcp.accept(listen)
+
+      :ok = :gen_tcp.send(clickhouse, intercept_packets(mint))
+      :ok = :gen_tcp.send(mint, intercept_packets(clickhouse))
+
+      spawn_link(fn ->
+        assert {:ok, %{num_rows: 1, rows: [[2]]}} = Ch.query(conn, "select 1 + 1")
+      end)
+
+      :ok = :gen_tcp.send(clickhouse, intercept_packets(mint))
+
+      response =
+        String.replace(
+          intercept_packets(clickhouse),
+          "Connection: Keep-Alive",
+          "Connection: Close"
+        )
+
+      :ok = :gen_tcp.send(mint, response)
+      :ok = :gen_tcp.close(mint)
+
+      # close the listening socket so reconnection will fail
+      :ok = :gen_tcp.close(listen)
+
+      log =
+        capture_async_log(fn ->
+          assert_raise DBConnection.ConnectionError, ~r/Reconnection failed/, fn ->
+            # this should trigger maybe_reconnect
+            Ch.query(conn, "select 1 + 1")
+          end
+        end)
+
+      assert log =~ "Could not reconnect due to %Mint.TransportError{reason: :econnrefused}"
+    end
   end
 
   defp first_byte(binary) do