Clean up Redix.PubSub.get_client_id/1

lib/redix/pubsub.ex

@@ -418,14 +418,24 @@ defmodule Redix.PubSub do
   end
 
   @doc """
-  Gets the redis `CLIENT ID` associated with a connection
+  Gets the Redis `CLIENT ID` associated with a connection.
+
+  This is useful for implementing [**client-side
+  caching**](https://redis.io/docs/manual/client-side-caching/), where you can
+  subscribe your pub/sub connection to changes on keys.
+
+  If the pub/sub connection is currently disconnected, this function returns
+  `{:error, error}`.
+
   ## Examples
 
-      iex> Redix.PubSub.client_id(conn)
+      iex> Redix.PubSub.get_client_id(conn)
       {:ok, 123}
+
   """
-  @spec client_id(connection()) :: {:ok, integer()} | {:error, any()}
-  def client_id(conn) do
-    :gen_statem.call(conn, :client_id)
+  @doc since: "1.4.0"
+  @spec get_client_id(connection()) :: {:ok, integer()} | {:error, Redix.ConnectionError.t()}
+  def get_client_id(conn) do
+    :gen_statem.call(conn, :get_client_id)
   end
 end

lib/redix/pubsub/connection.ex

@@ -194,10 +194,9 @@ defmodule Redix.PubSub.Connection do
     {:keep_state, data}
   end
 
-  def disconnected({:call, from}, :client_id, data) do
-    :ok = :gen_statem.reply(from, {:error, :disconnected})
-
-    {:keep_state, data}
+  def disconnected({:call, from}, :get_client_id, _data) do
+    reply = {:error, %ConnectionError{reason: :closed}}
+    {:keep_state_and_data, {:reply, from, reply}}
   end
 
   def connected(:internal, :handle_connection, data) do
@@ -231,10 +230,8 @@ defmodule Redix.PubSub.Connection do
     end
   end
 
-  def connected({:call, from}, :client_id, data) do
-    :ok = :gen_statem.reply(from, {:ok, data.client_id})
-
-    {:keep_state, data}
+  def connected({:call, from}, :get_client_id, data) do
+    {:keep_state_and_data, {:reply, from, {:ok, data.client_id}}}
   end
 
   def connected(:info, {transport_closed, socket}, %__MODULE__{socket: socket} = data)

test/redix/pubsub_test.exs

@@ -23,9 +23,15 @@ defmodule Redix.PubSubTest do
     assert info[:fullsweep_after] == fullsweep_after
   end
 
-  test "client_id should be available after start_link/2" do
+  test "client_id/1 should be available after start_link/2" do
     {:ok, pid} = PubSub.start_link(port: @port)
-    assert match?({:ok, client_id} when is_number(client_id), PubSub.client_id(pid))
+    assert {:ok, client_id} = PubSub.get_client_id(pid)
+    assert is_integer(client_id)
+  end
+
+  test "client_id/1 returns an error if connection fails" do
+    {:ok, pid} = PubSub.start_link(port: 9999, name: :redix_pubsub_telemetry_failed_conn_test)
+    assert {:error, %ConnectionError{reason: :closed}} = PubSub.get_client_id(pid)
   end
 
   test "subscribe/unsubscribe flow", %{pubsub: pubsub, conn: conn} do