Repository: elixir-grpc/grpc (elixir)

Issue:
Better error handling 
I think the error handling good be improved a little bit. Instead of always returning the same `RPCError` struct we could pattern match on the status code and return different errors. Currently I'm doing it like below. If you like this approach I could do a PR:

```elixir
defmodule Bossman.Job.Error do
  defmodule NotFound, do: defexception([:message])
  defmodule Unknown, do: defexception([:message])
  defmodule InvalidArgument, do: defexception([:message])

  def new(%GRPC.RPCError{message: message, status: status}) do
    status_to_error_struct(status, URI.decode(message))
  end

  defp status_to_error_struct(2, msg), do: %Unknown{message: msg}
  defp status_to_error_struct(3, msg), do: %InvalidArgument{message: msg}
  defp status_to_error_struct(5, msg), do: %NotFound{message: msg}
end
```



Pull request description:
Docs: Improves the module docs for error statuses.
* Adds a markdown table to detail each code status and its related message.
* Adds `is_rpc_error/2` guard clause to check if it's an RPCError struct.
* Add GRPC.Status and GRPC.RPCError unit tests

Closes #170

Patch:
diff --git a/lib/grpc/error.ex b/lib/grpc/error.ex
deleted file mode 100644
index c6aff4d..0000000
--- a/lib/grpc/error.ex
+++ /dev/null
@@ -1,88 +0,0 @@
-defmodule GRPC.RPCError do
-  @moduledoc """
-  The RPC error raised in server side and got in client side.
-
-      # server side
-      raise GRPC.RPCError, status: :unknown # preferred
-      raise GRPC.RPCError, status: GRPC.Status.unknown, message: "error message"
-
-      # client side
-      {:error, error} = Your.Stub.unary_call(channel, request)
-  """
-
-  defexception [:status, :message]
-  @type t :: %__MODULE__{status: GRPC.Status.t(), message: String.t()}
-
-  alias GRPC.Status
-
-  @spec exception(Status.t(), String.t()) :: t
-  def new(status) when is_atom(status) do
-    exception(status: status)
-  end
-
-  def exception(args) when is_list(args) do
-    error = parse_args(args, %__MODULE__{})
-
-    if error.message do
-      error
-    else
-      Map.put(error, :message, status_message(error.status))
-    end
-  end
-
-  defp parse_args([], acc), do: acc
-
-  defp parse_args([{:status, status} | t], acc) when is_integer(status) do
-    acc = %{acc | status: status}
-    parse_args(t, acc)
-  end
-
-  defp parse_args([{:status, status} | t], acc) when is_atom(status) do
-    acc = %{acc | status: apply(GRPC.Status, status, [])}
-    parse_args(t, acc)
-  end
-
-  defp parse_args([{:message, message} | t], acc) when is_binary(message) do
-    acc = %{acc | message: message}
-    parse_args(t, acc)
-  end
-
-  def exception(status, message) when is_atom(status) do
-    %GRPC.RPCError{status: apply(GRPC.Status, status, []), message: message}
-  end
-
-  def exception(status, message) when is_integer(status) do
-    %GRPC.RPCError{status: status, message: message}
-  end
-
-  defp status_message(1), do: "The operation was cancelled (typically by the caller)"
-  defp status_message(2), do: "Unknown error"
-  defp status_message(3), do: "Client specified an invalid argument"
-  defp status_message(4), do: "Deadline expired before operation could complete"
-  defp status_message(5), do: "Some requested entity (e.g., file or directory) was not found"
-
-  defp status_message(6),
-    do: "Some entity that we attempted to create (e.g., file or directory) already exists"
-
-  defp status_message(7),
-    do: "The caller does not have permission to execute the specified operation"
-
-  defp status_message(8), do: "Some resource has been exhausted"
-
-  defp status_message(9),
-    do:
-      "Operation was rejected because the system is not in a state required for the operation's execution"
-
-  defp status_message(10), do: "The operation was aborted"
-  defp status_message(11), do: "Operation was attempted past the valid range"
-
-  defp status_message(12),
-    do: "Operation is not implemented or not supported/enabled in this service"
-
-  defp status_message(13), do: "Internal errors"
-  defp status_message(14), do: "The service is currently unavailable"
-  defp status_message(15), do: "Unrecoverable data loss or corruption"
-
-  defp status_message(16),
-    do: "The request does not have valid authentication credentials for the operation"
-end
diff --git a/lib/grpc/rpc_error.ex b/lib/grpc/rpc_error.ex
new file mode 100644
index 0000000..39ec37e
--- /dev/null
+++ b/lib/grpc/rpc_error.ex
@@ -0,0 +1,99 @@
+defmodule GRPC.RPCError do
+  @moduledoc """
+  The RPC error raised in server side and got in client side.
+
+      # server side
+      raise GRPC.RPCError, status: :unknown # preferred
+      raise GRPC.RPCError, status: GRPC.Status.unknown, message: "error message"
+
+      # client side
+      {:error, error} = Your.Stub.unary_call(channel, request)
+
+  Error handling can be done with the `is_rpc_error/2` guard.
+
+  Expanding on the code above, the first option is for the guard to
+  be used in a `cond` or `case`, as follows:
+
+      cond do
+        is_rpc_error(error, GRPC.Status.not_found()) ->
+          do_something_when_not_found()
+
+        is_rpc_error(error, GRPC.Status.out_of_range()) ->
+          do_something_when_out_of_range()
+
+        true ->
+          fallback_code()
+      end
+
+  Another option is for the error handling to be written into a multi-clause function.
+  In such case we must define module attributes for each of the errors we want because
+  the functions in `GRPC.Status` can't be called directly inside the guard.
+
+      ...
+      handle_error(error)
+      ...
+
+      @not_found GRPC.Status.not_found()
+      @out_of_range GRPC.Status.out_of_range()
+
+      defp handle_error(error) when is_rpc_error(error, @not_found) do
+        # not found
+      end
+
+      defp handle_error(error) when is_rpc_error(error, @out_of_range) do
+        # out of range
+      end
+
+      defp handle_error(error) do
+        # fallback
+      end
+
+  See `GRPC.Status` for more details on possible statuses.
+  """
+
+  defexception [:status, :message]
+
+  defguard is_rpc_error(e, status) when is_struct(e, __MODULE__) and e.status == status
+
+  @type t :: %__MODULE__{status: GRPC.Status.t(), message: String.t()}
+
+  alias GRPC.Status
+
+  @spec new(status :: atom()) :: t()
+  def new(status) when is_atom(status) do
+    exception(status: status)
+  end
+
+  @spec exception(args :: list()) :: t()
+  def exception(args) when is_list(args) do
+    error = parse_args(args, %__MODULE__{})
+
+    %{error | message: error.message || Status.status_message(error.status)}
+  end
+
+  defp parse_args([], acc), do: acc
+
+  defp parse_args([{:status, status} | t], acc) when is_integer(status) do
+    acc = %{acc | status: status}
+    parse_args(t, acc)
+  end
+
+  defp parse_args([{:status, status} | t], acc) when is_atom(status) do
+    acc = %{acc | status: apply(GRPC.Status, status, [])}
+    parse_args(t, acc)
+  end
+
+  defp parse_args([{:message, message} | t], acc) when is_binary(message) do
+    acc = %{acc | message: message}
+    parse_args(t, acc)
+  end
+
+  @spec exception(status :: Status.t() | atom(), message :: String.t()) :: t()
+  def exception(status, message) when is_atom(status) do
+    %GRPC.RPCError{status: apply(GRPC.Status, status, []), message: message}
+  end
+
+  def exception(status, message) when is_integer(status) do
+    %GRPC.RPCError{status: status, message: message}
+  end
+end
diff --git a/lib/grpc/status.ex b/lib/grpc/status.ex
index 877d503..aa2651a 100644
--- a/lib/grpc/status.ex
+++ b/lib/grpc/status.ex
@@ -1,8 +1,28 @@
 defmodule GRPC.Status do
   @moduledoc """
-  Collection of gRPC status.
-
-  Please refer to https://github.com/grpc/grpc/blob/master/include/grpc/impl/codegen/status.h
+  A collection of gRPC statuses.
+
+  |  Code  |         Status           |                                            Status Message                                           |
+  |-------:|--------------------------|-----------------------------------------------------------------------------------------------------|
+  |   `0`  |  `:ok`                   |  Success                                                                                            |
+  |   `1`  |  `:cancelled`            |  The operation was cancelled (typically by the caller)                                              |
+  |   `2`  |  `:unknown`              |  Unknown error                                                                                      |
+  |   `3`  |  `:invalid_argument`     |  Client specified an invalid argument                                                               |
+  |   `4`  |  `:deadline_exceeded`    |  Deadline expired before operation could complete                                                   |
+  |   `5`  |  `:not_found`            |  Some requested entity (e.g., file or directory) was not found                                      |
+  |   `6`  |  `:already_exists`       |  Some entity that we attempted to create (e.g., file or directory) already exists                   |
+  |   `7`  |  `:permission_denied`    |  The caller does not have permission to execute the specified operation                             |
+  |   `8`  |  `:resource_exhausted`   |  Some resource has been exhausted                                                                   |
+  |   `9`  |  `:failed_precondition`  |  Operation was rejected because the system is not in a state required for the operation's execution |
+  |  `10`  |  `:aborted`              |  The operation was aborted                                                                          |
+  |  `11`  |  `:out_of_range`         |  Operation was attempted past the valid range                                                       |
+  |  `12`  |  `:unimplemented`        |  Operation is not implemented or not supported/enabled in this service                              |
+  |  `13`  |  `:internal`             |  Internal errors                                                                                    |
+  |  `14`  |  `:unavailable`          |  The service is currently unavailable                                                               |
+  |  `15`  |  `:data_loss`            |  Unrecoverable data loss or corruption                                                              |
+  |  `16`  |  `:unauthenticated`      |  The request does not have valid authentication credentials for the operation                       |
+
+  For more details, please refer to the [official gRPC docs](https://github.com/grpc/grpc/blob/master/doc/statuscodes.md)
   """
 
   @type t :: non_neg_integer
@@ -165,4 +185,37 @@ defmodule GRPC.Status do
   def code_name(14), do: "Unavailable"
   def code_name(15), do: "DataLoss"
   def code_name(16), do: "Unauthenticated"
+
+  @spec status_message(t()) :: String.t() | nil
+  def status_message(0), do: nil
+  def status_message(1), do: "The operation was cancelled (typically by the caller)"
+  def status_message(2), do: "Unknown error"
+  def status_message(3), do: "Client specified an invalid argument"
+  def status_message(4), do: "Deadline expired before operation could complete"
+  def status_message(5), do: "Some requested entity (e.g., file or directory) was not found"
+
+  def status_message(6),
+    do: "Some entity that we attempted to create (e.g., file or directory) already exists"
+
+  def status_message(7),
+    do: "The caller does not have permission to execute the specified operation"
+
+  def status_message(8), do: "Some resource has been exhausted"
+
+  def status_message(9),
+    do:
+      "Operation was rejected because the system is not in a state required for the operation's execution"
+
+  def status_message(10), do: "The operation was aborted"
+  def status_message(11), do: "Operation was attempted past the valid range"
+
+  def status_message(12),
+    do: "Operation is not implemented or not supported/enabled in this service"
+
+  def status_message(13), do: "Internal errors"
+  def status_message(14), do: "The service is currently unavailable"
+  def status_message(15), do: "Unrecoverable data loss or corruption"
+
+  def status_message(16),
+    do: "The request does not have valid authentication credentials for the operation"
 end
diff --git a/mix.exs b/mix.exs
index cc24060..0916296 100644
--- a/mix.exs
+++ b/mix.exs
@@ -44,7 +44,7 @@ defmodule GRPC.Mixfile do
       {:mint, "~> 1.4.2"},
       {:cowlib, "~> 2.12"},
       {:protobuf, "~> 0.11", only: [:dev, :test]},
-      {:ex_doc, "~> 0.28.6", only: :dev},
+      {:ex_doc, "~> 0.29", only: :dev},
       {:dialyxir, "~> 1.1.0", only: [:dev, :test], runtime: false},
       {:ex_parameterized, "~> 1.3.7", only: :test},
       {:telemetry, "~> 1.0"}
diff --git a/mix.lock b/mix.lock
index 690168b..1e028b6 100644
--- a/mix.lock
+++ b/mix.lock
@@ -5,7 +5,7 @@
   "dialyxir": {:hex, :dialyxir, "1.1.0", "c5aab0d6e71e5522e77beff7ba9e08f8e02bad90dfbeffae60eaf0cb47e29488", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "07ea8e49c45f15264ebe6d5b93799d4dd56a44036cf42d0ad9c960bc266c0b9a"},
   "earmark_parser": {:hex, :earmark_parser, "1.4.32", "fa739a0ecfa34493de19426681b23f6814573faee95dfd4b4aafe15a7b5b32c6", [:mix], [], "hexpm", "b8b0dd77d60373e77a3d7e8afa598f325e49e8663a51bcc2b88ef41838cca755"},
   "erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"},
-  "ex_doc": {:hex, :ex_doc, "0.28.6", "2bbd7a143d3014fc26de9056793e97600ae8978af2ced82c2575f130b7c0d7d7", [:mix], [{:earmark_parser, "~> 1.4.19", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "bca1441614654710ba37a0e173079273d619f9160cbcc8cd04e6bd59f1ad0e29"},
+  "ex_doc": {:hex, :ex_doc, "0.29.4", "6257ecbb20c7396b1fe5accd55b7b0d23f44b6aa18017b415cb4c2b91d997729", [:mix], [{:earmark_parser, "~> 1.4.31", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "2c6699a737ae46cb61e4ed012af931b57b699643b24dabe2400a8168414bc4f5"},
   "ex_parameterized": {:hex, :ex_parameterized, "1.3.7", "801f85fc4651cb51f11b9835864c6ed8c5e5d79b1253506b5bb5421e8ab2f050", [:mix], [], "hexpm", "1fb0dc4aa9e8c12ae23806d03bcd64a5a0fc9cd3f4c5602ba72561c9b54a625c"},
   "gun": {:hex, :gun, "2.0.1", "160a9a5394800fcba41bc7e6d421295cf9a7894c2252c0678244948e3336ad73", [:make, :rebar3], [{:cowlib, "2.12.1", [hex: :cowlib, repo: "hexpm", optional: false]}], "hexpm", "a10bc8d6096b9502205022334f719cc9a08d9adcfbfc0dbee9ef31b56274a20b"},
   "hpax": {:hex, :hpax, "0.1.2", "09a75600d9d8bbd064cdd741f21fc06fc1f4cf3d0fcc335e5aa19be1a7235c84", [:mix], [], "hexpm", "2c87843d5a23f5f16748ebe77969880e29809580efdaccd615cd3bed628a8c13"},


Test patch:
diff --git a/test/grpc/rpc_error_test.exs b/test/grpc/rpc_error_test.exs
new file mode 100644
index 0000000..f334582
--- /dev/null
+++ b/test/grpc/rpc_error_test.exs
@@ -0,0 +1,69 @@
+defmodule GRPC.RPCErrorTest do
+  use GRPC.DataCase, async: true
+
+  import GRPC.RPCError, only: [is_rpc_error: 2]
+
+  alias GRPC.Status
+  alias GRPC.RPCError
+
+  defmodule HandlingWithFunctionGuard do
+    @cancelled Status.cancelled()
+    @unknown Status.unknown()
+
+    def handle(err) when is_rpc_error(err, @cancelled) do
+      :cancelled
+    end
+
+    def handle(err) when is_rpc_error(err, @unknown) do
+      :unknown
+    end
+
+    def handle(_err) do
+      :fallback
+    end
+  end
+
+  describe "is_rpc_error/2 guard clause" do
+    test "returns true for GRPC.RPCError structs with same status" do
+      exception = GRPC.RPCError.new(:unknown)
+
+      assert is_rpc_error(exception, Status.unknown())
+    end
+
+    test "returns false for GRPC.RPCError structs with different status" do
+      exception = GRPC.RPCError.new(:unknown)
+
+      refute is_rpc_error(exception, Status.cancelled())
+    end
+
+    test "returns false for others types of input" do
+      exception = %{status: :unknown}
+
+      refute is_rpc_error(exception, Status.unknown())
+    end
+
+    test "supports clause guarding in function heads" do
+      assert HandlingWithFunctionGuard.handle(GRPC.RPCError.new(:unknown)) == :unknown
+      assert HandlingWithFunctionGuard.handle(GRPC.RPCError.new(:cancelled)) == :cancelled
+      assert HandlingWithFunctionGuard.handle(GRPC.RPCError.new(:not_found)) == :fallback
+    end
+  end
+
+  describe "new/1" do
+    test "creates the GRPC.RPCError struct with a default message" do
+      assert %RPCError{status: code, message: message} = RPCError.new(:unknown)
+      assert code == Status.unknown()
+      refute is_nil(message)
+    end
+
+    test "creates the GRPC.RPCError struct with its own predefined message and status code" do
+      # small sampling of statuses. ok must be here because its an edge case
+      for status_name <- [:ok, :cancelled, :unknown] do
+        status = apply(Status, status_name, [])
+
+        assert %RPCError{status: status, message: Status.status_message(status)} ==
+                 RPCError.new(status_name)
+      end
+    end
+  end
+end
diff --git a/test/grpc/status_test.exs b/test/grpc/status_test.exs
new file mode 100644
index 0000000..0074a81
--- /dev/null
+++ b/test/grpc/status_test.exs
@@ -0,0 +1,50 @@
+defmodule GRPC.StatusTest do
+  use GRPC.DataCase, async: true
+
+  alias GRPC.Status
+
+  @status_codes_metadata [
+    {:ok, 0, nil},
+    {:cancelled, 1, "The operation was cancelled (typically by the caller)"},
+    {:unknown, 2, "Unknown error"},
+    {:invalid_argument, 3, "Client specified an invalid argument"},
+    {:deadline_exceeded, 4, "Deadline expired before operation could complete"},
+    {:not_found, 5, "Some requested entity (e.g., file or directory) was not found"},
+    {:already_exists, 6,
+     "Some entity that we attempted to create (e.g., file or directory) already exists"},
+    {:permission_denied, 7,
+     "The caller does not have permission to execute the specified operation"},
+    {:resource_exhausted, 8, "Some resource has been exhausted"},
+    {:failed_precondition, 9,
+     "Operation was rejected because the system is not in a state required for the operation's execution"},
+    {:aborted, 10, "The operation was aborted"},
+    {:out_of_range, 11, "Operation was attempted past the valid range"},
+    {:unimplemented, 12, "Operation is not implemented or not supported/enabled in this service"},
+    {:internal, 13, "Internal errors"},
+    {:unavailable, 14, "The service is currently unavailable"},
+    {:data_loss, 15, "Unrecoverable data loss or corruption"},
+    {:unauthenticated, 16,
+     "The request does not have valid authentication credentials for the operation"}
+  ]
+
+  for {function, expected_code, expected_message} <- @status_codes_metadata do
+    test "function #{function} returns expected code" do
+      assert unquote(expected_code) == Status.unquote(function)()
+    end
+
+    test "code #{expected_code} returns the expected message" do
+      assert unquote(expected_message) == Status.status_message(unquote(expected_code))
+    end
+
+    test "code #{expected_code} returns the expected code_name" do
+      code_name =
+        case unquote(function) do
+          :ok -> "OK"
+          :cancelled -> "Canceled"
+          function -> function |> to_string() |> Macro.camelize()
+        end
+
+      assert code_name == Status.code_name(unquote(expected_code))
+    end
+  end
+end