From 1918de765633edbc8b18eea10cb5ff40c55740fc Mon Sep 17 00:00:00 2001
From: zachdaniel <zachary.s.daniel@gmail.com>
Date: Wed, 3 Apr 2019 13:09:53 -0400
Subject: [PATCH] feat: slightly more informative json rpc error messages +
 logging

---
 .../controllers/api/rpc/rpc_translator.ex     | 50 ++++++++++++++-----
 .../api/rpc/rpc_translator_test.exs           | 16 ------
 2 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/rpc_translator.ex b/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/rpc_translator.ex
index b1db31a8f3..a5a4959e8d 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/rpc_translator.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/rpc_translator.ex
@@ -13,6 +13,8 @@ defmodule BlockScoutWeb.API.RPC.RPCTranslator do
 
   """
 
+  require Logger
+
   import Plug.Conn
   import Phoenix.Controller, only: [put_view: 2]
 
@@ -28,12 +30,28 @@ defmodule BlockScoutWeb.API.RPC.RPCTranslator do
          {:ok, conn} <- call_controller(conn, controller, action) do
       conn
     else
-      _ ->
+      {:error, :no_action} ->
         conn
         |> put_status(400)
         |> put_view(RPCView)
         |> Controller.render(:error, error: "Unknown action")
         |> halt()
+
+      {:error, error} ->
+        Logger.error(fn -> ["Error while calling RPC action", inspect(error)] end)
+
+        conn
+        |> put_status(500)
+        |> put_view(RPCView)
+        |> Controller.render(:error, error: "Something went wrong.")
+        |> halt()
+
+      _ ->
+        conn
+        |> put_status(500)
+        |> put_view(RPCView)
+        |> Controller.render(:error, error: "Something went wrong.")
+        |> halt()
     end
   end
 
@@ -46,27 +64,35 @@ defmodule BlockScoutWeb.API.RPC.RPCTranslator do
   end
 
   @doc false
-  @spec translate_module(map(), String.t()) :: {:ok, module()} | :error
-  def translate_module(translations, module) do
+  @spec translate_module(map(), String.t()) :: {:ok, module()} | {:error, :no_action}
+  defp translate_module(translations, module) do
     module_lowercase = String.downcase(module)
-    Map.fetch(translations, module_lowercase)
+
+    case Map.fetch(translations, module_lowercase) do
+      {:ok, module} -> {:ok, module}
+      _ -> {:error, :no_action}
+    end
   end
 
   @doc false
-  @spec translate_action(String.t()) :: {:ok, atom()} | :error
-  def translate_action(action) do
+  @spec translate_action(String.t()) :: {:ok, atom()} | {:error, :no_action}
+  defp translate_action(action) do
     action_lowercase = String.downcase(action)
     {:ok, String.to_existing_atom(action_lowercase)}
   rescue
-    ArgumentError -> :error
+    ArgumentError -> {:error, :no_action}
   end
 
   @doc false
-  @spec call_controller(Conn.t(), module(), atom()) :: {:ok, Conn.t()} | :error
-  def call_controller(conn, controller, action) do
-    {:ok, controller.call(conn, action)}
+  @spec call_controller(Conn.t(), module(), atom()) :: {:ok, Conn.t()} | {:error, :no_action} | {:error, Exception.t()}
+  defp call_controller(conn, controller, action) do
+    if :erlang.function_exported(controller, action, 2) do
+      {:ok, controller.call(conn, action)}
+    else
+      {:error, :no_action}
+    end
   rescue
-    Conn.WrapperError ->
-      :error
+    e ->
+      {:error, Exception.format(:error, e, __STACKTRACE__)}
   end
 end
diff --git a/apps/block_scout_web/test/block_scout_web/controllers/api/rpc/rpc_translator_test.exs b/apps/block_scout_web/test/block_scout_web/controllers/api/rpc/rpc_translator_test.exs
index 6a847bbc5b..3300085679 100644
--- a/apps/block_scout_web/test/block_scout_web/controllers/api/rpc/rpc_translator_test.exs
+++ b/apps/block_scout_web/test/block_scout_web/controllers/api/rpc/rpc_translator_test.exs
@@ -75,20 +75,4 @@ defmodule BlockScoutWeb.API.RPC.RPCTranslatorTest do
       assert json_response(result, 200) == %{}
     end
   end
-
-  test "translate_module/2" do
-    assert RPCTranslator.translate_module(%{"test" => __MODULE__}, "tesT") == {:ok, __MODULE__}
-    assert RPCTranslator.translate_module(%{}, "test") == :error
-  end
-
-  test "translate_action/1" do
-    expected = :test_atom
-    assert RPCTranslator.translate_action("test_atoM") == {:ok, expected}
-    assert RPCTranslator.translate_action("some_atom_that_should_not_exist") == :error
-  end
-
-  test "call_controller/3", %{conn: conn} do
-    assert RPCTranslator.call_controller(conn, TestController, :bad_action) == :error
-    assert {:ok, %Plug.Conn{}} = RPCTranslator.call_controller(conn, TestController, :test_action)
-  end
 end