Better error handling for failure to connect to Redis server

lib/srh/auth/token_resolver.ex

@@ -70,11 +70,13 @@ defmodule Srh.Auth.TokenResolver do
     {srh_max_connections, ""} = Integer.parse(System.get_env("SRH_MAX_CONNECTIONS", "3"))
 
     # Create a config-file-like structure that the ETS layout expects, with just one entry
-    config_file_data = Map.put(%{}, srh_token, %{
-      "srh_id" => "env_config_connection", # Jason.parse! expects these keys to be strings, not atoms, so we need to replicate that setup
-      "connection_string" => srh_connection_string,
-      "max_connections" => srh_max_connections
-    })
+    config_file_data =
+      Map.put(%{}, srh_token, %{
+        # Jason.parse! expects these keys to be strings, not atoms, so we need to replicate that setup
+        "srh_id" => "env_config_connection",
+        "connection_string" => srh_connection_string,
+        "max_connections" => srh_max_connections
+      })
 
     IO.puts("Loaded config from env. #{map_size(config_file_data)} entries.")
     # Load this into ETS
@@ -98,17 +100,17 @@ defmodule Srh.Auth.TokenResolver do
   # The env strategy uses the same ETS table as the file strategy, so we can fall back on that
   defp do_resolve("env", token), do: do_resolve("file", token)
 
-  defp do_resolve("redis", _token) do
-    {
-      :ok,
-      # This is done to replicate what will eventually be API endpoints, so they keys are not atoms
-      Jason.decode!(
-        Jason.encode!(%{
-          srh_id: "1000",
-          connection_string: "redis://localhost:6379",
-          max_connections: 10
-        })
-      )
-    }
-  end
+  #  defp do_resolve("redis", _token) do
+  #    {
+  #      :ok,
+  #      # This is done to replicate what will eventually be API endpoints, so they keys are not atoms
+  #      Jason.decode!(
+  #        Jason.encode!(%{
+  #          srh_id: "1000",
+  #          connection_string: "redis://localhost:6379",
+  #          max_connections: 10
+  #        })
+  #      )
+  #    }
+  #  end
 end

lib/srh/http/base_router.ex

@@ -54,7 +54,8 @@ defmodule Srh.Http.BaseRouter do
          |> get_req_header("upstash-encoding")
          |> RequestValidator.validate_encoding_header() do
       {:ok, _encoding_enabled} -> true
-      {:error, _} -> false # it's not required to be present
+      # it's not required to be present
+      {:error, _} -> false
     end
   end
 
@@ -63,7 +64,9 @@ defmodule Srh.Http.BaseRouter do
       true ->
         # We need to use the encoder to
         ResultEncoder.encode_response(response)
-      false -> response
+
+      false ->
+        response
     end
   end
 
@@ -85,6 +88,9 @@ defmodule Srh.Http.BaseRouter do
         {:not_authorized, message} ->
           %{code: 401, message: message, json: false}
 
+        {:connection_error, message} ->
+          %{code: 500, message: message, json: false}
+
         {:server_error, _} ->
           %{code: 500, message: "An error occurred internally", json: false}
 

lib/srh/http/command_handler.ex

@@ -99,39 +99,43 @@ defmodule Srh.Http.CommandHandler do
         do_dispatch_command_transaction_array(wrapped_command_array, worker_pid, responses)
 
         # Now manually run the EXEC - this is what contains the information to form the response, not the above
-        result = case ClientWorker.redis_command(worker_pid, ["EXEC"]) do
-          {:ok, res} ->
-            {
-              :ok,
-              res
-              |> Enum.map(&(%{result: &1}))
-            }
-          # TODO: Can there be any inline errors here? Wouldn't they fail the whole tx?
+        result =
+          case ClientWorker.redis_command(worker_pid, ["EXEC"]) do
+            {:ok, res} ->
+              {
+                :ok,
+                res
+                |> Enum.map(&%{result: &1})
+              }
 
-          {:error, error} ->
-            {:redis_error, %{error: error.message}}
-        end
+            {:error, error} ->
+              decode_error(error)
+          end
 
         Client.return_worker(client_pid, worker_pid)
 
         result
+
       {:error, msg} ->
         {:server_error, msg}
     end
   end
 
-  defp do_dispatch_command_transaction_array([current | rest], worker_pid, responses) when is_pid(worker_pid) do
-    updated_responses = case ClientWorker.redis_command(worker_pid, current) do
-      {:ok, res} ->
-        [%{result: res} | responses]
-
-      {:error, error} ->
-        [
-          %{
-            error: error.message
-          } | responses
-        ]
-    end
+  defp do_dispatch_command_transaction_array([current | rest], worker_pid, responses)
+       when is_pid(worker_pid) do
+    updated_responses =
+      case ClientWorker.redis_command(worker_pid, current) do
+        {:ok, res} ->
+          [%{result: res} | responses]
+
+        {:error, error} ->
+          [
+            %{
+              error: error.message
+            }
+            | responses
+          ]
+      end
 
     do_dispatch_command_transaction_array(rest, worker_pid, updated_responses)
   end
@@ -154,16 +158,30 @@ defmodule Srh.Http.CommandHandler do
             {:ok, %{result: res}}
 
           {:error, error} ->
-            {
-              :redis_error,
-              %{
-                error: error.message
-              }
-            }
+            decode_error(error)
         end
 
       {:error, msg} ->
         {:server_error, msg}
     end
   end
+
+  # Figure out if it's an actual Redis error or a Redix error
+  defp decode_error(error) do
+    case error do
+      %{reason: :closed} ->
+        {
+          :connection_error,
+          "Unable to connect to the Redis server"
+        }
+
+      _ ->
+        {
+          :redis_error,
+          %{
+            error: error.message
+          }
+        }
+    end
+  end
 end

lib/srh/redis/client_worker.ex

@@ -52,7 +52,6 @@ defmodule Srh.Redis.ClientWorker do
     {:noreply, state}
   end
 
-  # TODO: Handle host / port connections
   def handle_info(
         :create_connection,
         %{
@@ -62,6 +61,7 @@ defmodule Srh.Redis.ClientWorker do
         } = state
       )
       when is_binary(connection_string) do
+    # Will cause a crash for this genserver if the connection fails
     {:ok, pid} = Redix.start_link(connection_string)
     {:noreply, %{state | redix_pid: pid}}
   end