Avoid catching Xandra errors, let RPC take care of temporary failures

This change avoids catching and managing Xandra errors within the code
dedicated to interact with the database, where the concern does not
require handling expections.

Exceptions and temporary failures are better handled by whomever calls
Pairing functionality: in this case RPC, in the future it might be
directly a Phoenix conn handling a request.

Note that Astarte RPC already handles failures by responding with a
generic error to the caller.
The exception is logged for operational purposes, so developers can
troubleshoot the issue, but these kind of database errors are not of
much use to API users.

Signed-off-by: Davide Briani <davide.briani@secomind.com>

Author: davidebriani

apps/astarte_pairing/lib/astarte_pairing/queries.ex

@@ -30,53 +30,16 @@ defmodule Astarte.Pairing.Queries do
   require Logger
   import Ecto.Query
 
-  @keyspace_does_not_exist_regex ~r/Keyspace (.*) does not exist/
-
   def get_agent_public_key_pems(realm_name) do
     keyspace = CQLUtils.realm_name_to_keyspace_name(realm_name, Config.astarte_instance_id!())
 
-    try do
-      with {:ok, pem} <-
-             KvStore.fetch_value("auth", "jwt_public_key_pem", :string,
-               prefix: keyspace,
-               consistency: :quorum,
-               error: :public_key_not_found
-             ) do
-        {:ok, [pem]}
-      end
-    rescue
-      err -> handle_xandra_error(err)
-    end
-  end
-
-  defp handle_xandra_error(%Xandra.ConnectionError{} = error) do
-    _ =
-      Logger.warning("Database connection error #{Exception.message(error)}.",
-        tag: "database_connection_error"
-      )
-
-    {:error, :database_connection_error}
-  end
-
-  defp handle_xandra_error(%Xandra.Error{} = error) do
-    %Xandra.Error{message: message} = error
-
-    case Regex.run(@keyspace_does_not_exist_regex, message) do
-      [_message, keyspace] ->
-        Logger.warning("Keyspace #{keyspace} does not exist.",
-          tag: "realm_not_found"
-        )
-
-        {:error, :realm_not_found}
-
-      nil ->
-        _ =
-          Logger.warning(
-            "Database error: #{Exception.message(error)}.",
-            tag: "database_error"
-          )
-
-        {:error, :database_error}
+    with {:ok, pem} <-
+           KvStore.fetch_value("auth", "jwt_public_key_pem", :string,
+             prefix: keyspace,
+             consistency: :quorum,
+             error: :public_key_not_found
+           ) do
+      {:ok, [pem]}
     end
   end
 
@@ -155,15 +118,11 @@ defmodule Astarte.Pairing.Queries do
     keyspace_name =
       CQLUtils.realm_name_to_keyspace_name(realm_name, Config.astarte_instance_id!())
 
-    try do
-      Repo.fetch(Device, device_id,
-        prefix: keyspace_name,
-        consistency: :quorum,
-        error: :device_not_found
-      )
-    rescue
-      err -> handle_xandra_error(err)
-    end
+    Repo.fetch(Device, device_id,
+      prefix: keyspace_name,
+      consistency: :quorum,
+      error: :device_not_found
+    )
   end
 
   def update_device_after_credentials_request(realm_name, device, cert_data, device_ip, nil) do
@@ -201,42 +160,34 @@ defmodule Astarte.Pairing.Queries do
   def fetch_device_registration_limit(realm_name) do
     keyspace = CQLUtils.realm_name_to_keyspace_name("astarte", Config.astarte_instance_id!())
 
-    try do
-      case Repo.fetch(Realm, realm_name,
-             prefix: keyspace,
-             consistency: :one,
-             error: :realm_not_found
-           ) do
-        {:ok, realm} ->
-          {:ok, realm.device_registration_limit}
-
-        {:error, :realm_not_found} ->
-          Logger.warning(
-            "cannot fetch device registration limit: realm #{realm_name} not found",
-            tag: "realm_not_found"
-          )
+    case Repo.fetch(Realm, realm_name,
+           prefix: keyspace,
+           consistency: :one,
+           error: :realm_not_found
+         ) do
+      {:ok, realm} ->
+        {:ok, realm.device_registration_limit}
+
+      {:error, :realm_not_found} ->
+        Logger.warning(
+          "cannot fetch device registration limit: realm #{realm_name} not found",
+          tag: "realm_not_found"
+        )
 
-          {:error, :realm_not_found}
-      end
-    rescue
-      err -> handle_xandra_error(err)
+        {:error, :realm_not_found}
     end
   end
 
   def fetch_registered_devices_count(realm_name) do
     keyspace =
       CQLUtils.realm_name_to_keyspace_name(realm_name, Config.astarte_instance_id!())
 
-    try do
-      count =
-        Device
-        |> select([d], count())
-        |> Repo.one!(prefix: keyspace, consistency: :one)
+    count =
+      Device
+      |> select([d], count())
+      |> Repo.one!(prefix: keyspace, consistency: :one)
 
-      {:ok, count}
-    rescue
-      err -> handle_xandra_error(err)
-    end
+    {:ok, count}
   end
 
   defp do_register_device(

apps/astarte_pairing/test/astarte_pairing/engine_test.exs

@@ -63,7 +63,9 @@ defmodule Astarte.Pairing.EngineTest do
 
   describe "get_agent_public_key_pem" do
     test "fails with non-existing realm" do
-      assert {:error, :realm_not_found} = Engine.get_agent_public_key_pems("nonexisting")
+      assert_raise Xandra.Error, "Keyspace nonexisting does not exist", fn ->
+        Engine.get_agent_public_key_pems("nonexisting")
+      end
     end
 
     test "successful call" do
@@ -214,7 +216,9 @@ defmodule Astarte.Pairing.EngineTest do
       realm = "nonexisting"
       device_id = TestHelper.random_128_bit_hw_id()
 
-      assert {:error, :realm_not_found} = Engine.unregister_device(realm, device_id)
+      assert_raise Xandra.Error, "Keyspace nonexisting does not exist", fn ->
+        Engine.unregister_device(realm, device_id)
+      end
     end
 
     test "fails with invalid device_id" do
@@ -292,15 +296,16 @@ defmodule Astarte.Pairing.EngineTest do
     test "fails with unexisting realm", %{hw_id: hw_id, secret: secret} do
       realm = "unexisting"
 
-      assert {:error, :realm_not_found} =
-               Engine.get_credentials(
-                 @astarte_protocol,
-                 @astarte_credentials_params,
-                 realm,
-                 hw_id,
-                 secret,
-                 @valid_ip
-               )
+      assert_raise Xandra.Error, "Keyspace unexisting does not exist", fn ->
+        Engine.get_credentials(
+          @astarte_protocol,
+          @astarte_credentials_params,
+          realm,
+          hw_id,
+          secret,
+          @valid_ip
+        )
+      end
     end
 
     test "fails with not registered device" do

apps/astarte_pairing/test/astarte_pairing/rpc/handler_test.exs

@@ -127,23 +127,9 @@ defmodule Astarte.Pairing.RPC.HandlerTest do
         %Call{call: {:get_agent_public_key_pems, %GetAgentPublicKeyPEMs{realm: "invalid"}}}
         |> Call.encode()
 
-      {:ok, reply} = Handler.handle_rpc(encoded)
-
-      expected_err_reply = %Reply{
-        error: true,
-        reply:
-          {:generic_error_reply,
-           %GenericErrorReply{
-             error_name: "realm_not_found",
-             error_data: nil,
-             user_readable_error_name: nil,
-             user_readable_message: nil,
-             user_readable_message: nil
-           }},
-        version: 0
-      }
-
-      assert Reply.decode(reply) == expected_err_reply
+      assert_raise Xandra.Error, "Keyspace invalid does not exist", fn ->
+        Handler.handle_rpc(encoded)
+      end
     end
 
     test "successful call" do
@@ -183,23 +169,9 @@ defmodule Astarte.Pairing.RPC.HandlerTest do
         %Call{call: {:get_info, %GetInfo{realm: "invalid", hw_id: hw_id, secret: secret}}}
         |> Call.encode()
 
-      {:ok, reply} = Handler.handle_rpc(encoded)
-
-      expected_err_reply = %Reply{
-        error: true,
-        reply:
-          {:generic_error_reply,
-           %GenericErrorReply{
-             error_name: "realm_not_found",
-             error_data: nil,
-             user_readable_error_name: nil,
-             user_readable_message: nil,
-             user_readable_message: nil
-           }},
-        version: 0
-      }
-
-      assert Reply.decode(reply) == expected_err_reply
+      assert_raise Xandra.Error, "Keyspace invalid does not exist", fn ->
+        Handler.handle_rpc(encoded)
+      end
     end
 
     test "successful call with pending device", %{hw_id: hw_id, secret: secret} do