From 27ae8e41e51119198c7a44b5e43ccc976fac67a3 Mon Sep 17 00:00:00 2001 From: Rishab Jaggi Date: Fri, 10 Sep 2021 15:19:41 -0400 Subject: [PATCH] Add details to grpc error status struct --- lib/grpc/adapter/cowboy/handler.ex | 35 +++++++++++++++++++-------- lib/grpc/adapter/gun.ex | 12 +++++++-- lib/grpc/error.ex | 31 +++++++++++++++--------- lib/grpc/stub.ex | 11 ++++++++- lib/grpc/transport/http2.ex | 7 +++--- test/grpc/integration/codec_test.exs | 3 ++- test/grpc/integration/server_test.exs | 15 ++++++++---- test/grpc/integration/stub_test.exs | 3 ++- 8 files changed, 83 insertions(+), 34 deletions(-) diff --git a/lib/grpc/adapter/cowboy/handler.ex b/lib/grpc/adapter/cowboy/handler.ex index f281f843..580c8dc3 100644 --- a/lib/grpc/adapter/cowboy/handler.ex +++ b/lib/grpc/adapter/cowboy/handler.ex @@ -54,7 +54,7 @@ defmodule GRPC.Adapter.Cowboy.Handler do {:cowboy_loop, req, %{pid: pid, handling_timer: timer_ref}} else {:error, error} -> - trailers = HTTP2.server_trailers(error.status, error.message) + trailers = HTTP2.server_trailers(error.status, error.message, error.details) req = send_error_trailers(req, trailers) {:ok, req, state} end @@ -255,8 +255,13 @@ defmodule GRPC.Adapter.Cowboy.Handler do end def info({:handling_timeout, _}, req, state = %{pid: pid}) do - error = %RPCError{status: GRPC.Status.deadline_exceeded(), message: "Deadline expired"} - trailers = HTTP2.server_trailers(error.status, error.message) + error = %RPCError{ + status: GRPC.Status.deadline_exceeded(), + message: "Deadline expired", + details: [] + } + + trailers = HTTP2.server_trailers(error.status, error.message, error.details) exit_handler(pid, :timeout) req = send_error_trailers(req, trailers) {:stop, req, state} @@ -280,7 +285,7 @@ defmodule GRPC.Adapter.Cowboy.Handler do # expected error raised from user to return error immediately def info({:EXIT, pid, {%RPCError{} = error, _stacktrace}}, req, state = %{pid: pid}) do - trailers = HTTP2.server_trailers(error.status, error.message) + trailers = HTTP2.server_trailers(error.status, error.message, error.details) exit_handler(pid, :rpc_error) req = send_error_trailers(req, trailers) {:stop, req, state} @@ -288,8 +293,13 @@ defmodule GRPC.Adapter.Cowboy.Handler do # unknown error raised from rpc def info({:EXIT, pid, {:handle_error, _kind}}, req, state = %{pid: pid}) do - error = %RPCError{status: GRPC.Status.unknown(), message: "Internal Server Error"} - trailers = HTTP2.server_trailers(error.status, error.message) + error = %RPCError{ + status: GRPC.Status.unknown(), + message: "Internal Server Error", + details: [] + } + + trailers = HTTP2.server_trailers(error.status, error.message, error.details) exit_handler(pid, :error) req = send_error_trailers(req, trailers) {:stop, req, state} @@ -297,8 +307,14 @@ defmodule GRPC.Adapter.Cowboy.Handler do def info({:EXIT, pid, {reason, stacktrace}}, req, state = %{pid: pid}) do Logger.error(Exception.format(:error, reason, stacktrace)) - error = %RPCError{status: GRPC.Status.unknown(), message: "Internal Server Error"} - trailers = HTTP2.server_trailers(error.status, error.message) + + error = %RPCError{ + status: GRPC.Status.unknown(), + message: "Internal Server Error", + details: [] + } + + trailers = HTTP2.server_trailers(error.status, error.message, error.details) exit_handler(pid, reason) req = send_error_trailers(req, trailers) {:stop, req, state} @@ -428,8 +444,7 @@ defmodule GRPC.Adapter.Cowboy.Handler do defp send_error(req, %{pid: pid}, msg) do error = RPCError.exception(status: :internal, message: msg) - trailers = HTTP2.server_trailers(error.status, error.message) - + trailers = HTTP2.server_trailers(error.status, error.message, error.details) exit_handler(pid, :rpc_error) send_error_trailers(req, trailers) end diff --git a/lib/grpc/adapter/gun.ex b/lib/grpc/adapter/gun.ex index bd7234aa..815f2ffb 100644 --- a/lib/grpc/adapter/gun.ex +++ b/lib/grpc/adapter/gun.ex @@ -196,7 +196,11 @@ defmodule GRPC.Adapter.Gun do {:error, GRPC.RPCError.exception( String.to_integer(headers["grpc-status"]), - headers["grpc-message"] + headers["grpc-message"], + headers["grpc-details"] + |> String.split(",") + |> Enum.filter(fn x -> x != "" end) + |> Enum.map(&Base.decode64!/1) )} end else @@ -215,7 +219,11 @@ defmodule GRPC.Adapter.Gun do {:error, GRPC.RPCError.exception( String.to_integer(headers["grpc-status"]), - headers["grpc-message"] + headers["grpc-message"], + headers["grpc-details"] + |> String.split(",") + |> Enum.filter(fn x -> x != "" end) + |> Enum.map(&Base.decode64!/1) )} else {:response, headers, :nofin} diff --git a/lib/grpc/error.ex b/lib/grpc/error.ex index c6aff4dc..eeb15667 100644 --- a/lib/grpc/error.ex +++ b/lib/grpc/error.ex @@ -5,17 +5,18 @@ defmodule GRPC.RPCError do # server side raise GRPC.RPCError, status: :unknown # preferred raise GRPC.RPCError, status: GRPC.Status.unknown, message: "error message" + raise GRPC.RPCError, status: GRPC.Status.unknown, details: [Google.Rpc.LocalizedMessage.new!(locale: “en-US”, message: “User friendly string”)] # client side {:error, error} = Your.Stub.unary_call(channel, request) """ - defexception [:status, :message] - @type t :: %__MODULE__{status: GRPC.Status.t(), message: String.t()} + defexception [:status, :message, :details] + @type t :: %__MODULE__{status: GRPC.Status.t(), message: String.t(), details: [any()]} alias GRPC.Status - @spec exception(Status.t(), String.t()) :: t + @spec exception(Status.t(), String.t(), [any()]) :: t def new(status) when is_atom(status) do exception(status: status) end @@ -23,10 +24,11 @@ defmodule GRPC.RPCError do 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)) + cond do + error.message && error.details -> error + error.details -> Map.put(error, :message, status_message(error.status)) + error.message -> Map.put(error, :details, []) + true -> error |> Map.put(:message, status_message(error.status)) |> Map.put(:details, []) end end @@ -47,12 +49,19 @@ defmodule GRPC.RPCError do parse_args(t, acc) end - def exception(status, message) when is_atom(status) do - %GRPC.RPCError{status: apply(GRPC.Status, status, []), message: message} + defp parse_args([{:details, details} | t], acc) do + acc = %{acc | details: details} + parse_args(t, acc) + end + + def exception(status, message, details \\ []) + + def exception(status, message, details) when is_atom(status) do + %GRPC.RPCError{status: apply(GRPC.Status, status, []), message: message, details: details} end - def exception(status, message) when is_integer(status) do - %GRPC.RPCError{status: status, message: message} + def exception(status, message, details) when is_integer(status) do + %GRPC.RPCError{status: status, message: message, details: details} end defp status_message(1), do: "The operation was cancelled (typically by the caller)" diff --git a/lib/grpc/stub.ex b/lib/grpc/stub.ex index b77bb5fa..9c9cd96c 100644 --- a/lib/grpc/stub.ex +++ b/lib/grpc/stub.ex @@ -501,7 +501,16 @@ defmodule GRPC.Stub do if status == GRPC.Status.ok() do :ok else - {:error, %GRPC.RPCError{status: status, message: trailers["grpc-message"]}} + {:error, + %GRPC.RPCError{ + status: status, + message: trailers["grpc-message"], + details: + trailers["grpc-details"] + |> String.split(",") + |> Enum.filter(fn x -> x != "" end) + |> Enum.map(&Base.decode64!/1) + }} end end diff --git a/lib/grpc/transport/http2.ex b/lib/grpc/transport/http2.ex index a5cbf248..660bd1da 100644 --- a/lib/grpc/transport/http2.ex +++ b/lib/grpc/transport/http2.ex @@ -12,11 +12,12 @@ defmodule GRPC.Transport.HTTP2 do %{"content-type" => "application/grpc+#{codec.name}"} end - @spec server_trailers(integer, String.t()) :: map - def server_trailers(status \\ Status.ok(), message \\ "") do + @spec server_trailers(integer, String.t(), [any()]) :: map + def server_trailers(status \\ Status.ok(), message \\ "", details \\ []) do %{ "grpc-status" => Integer.to_string(status), - "grpc-message" => message + "grpc-message" => message, + "grpc-details" => details |> Enum.map(&Base.encode64/1) |> Enum.join(",") } end diff --git a/test/grpc/integration/codec_test.exs b/test/grpc/integration/codec_test.exs index ef28b96a..65d35ca8 100644 --- a/test/grpc/integration/codec_test.exs +++ b/test/grpc/integration/codec_test.exs @@ -49,7 +49,8 @@ defmodule GRPC.Integration.CodecTest do assert %GRPC.RPCError{ status: GRPC.Status.unimplemented(), - message: "No codec registered for content-type application/grpc+not-registered" + message: "No codec registered for content-type application/grpc+not-registered", + details: [] } == reply end) end diff --git a/test/grpc/integration/server_test.exs b/test/grpc/integration/server_test.exs index 3960bf10..d52e230a 100644 --- a/test/grpc/integration/server_test.exs +++ b/test/grpc/integration/server_test.exs @@ -41,7 +41,7 @@ defmodule GRPC.Integration.ServerTest do end def say_hello(_req, _stream) do - raise GRPC.RPCError, status: GRPC.Status.unauthenticated(), message: "Please authenticate" + raise GRPC.RPCError, status: GRPC.Status.unauthenticated(), message: "Please authenticate", details: [123] end end @@ -109,7 +109,8 @@ defmodule GRPC.Integration.ServerTest do assert %GRPC.RPCError{ status: GRPC.Status.unauthenticated(), - message: "Please authenticate" + message: "Please authenticate", + details: [] } == reply end) end @@ -120,7 +121,11 @@ defmodule GRPC.Integration.ServerTest do req = Helloworld.HelloRequest.new(name: "unknown error") assert {:error, - %GRPC.RPCError{message: "Internal Server Error", status: GRPC.Status.unknown()}} == + %GRPC.RPCError{ + message: "Internal Server Error", + status: GRPC.Status.unknown(), + details: [] + }} == channel |> Helloworld.Greeter.Stub.say_hello(req) end) end @@ -129,7 +134,7 @@ defmodule GRPC.Integration.ServerTest do run_server([FeatureErrorServer], fn port -> {:ok, channel} = GRPC.Stub.connect("localhost:#{port}") rect = Routeguide.Rectangle.new() - error = %GRPC.RPCError{message: "Please authenticate", status: 16} + error = %GRPC.RPCError{message: "Please authenticate", status: 16, details: []} assert {:error, ^error} = channel |> Routeguide.RouteGuide.Stub.list_features(rect) end) end @@ -148,7 +153,7 @@ defmodule GRPC.Integration.ServerTest do run_server([TimeoutServer], fn port -> {:ok, channel} = GRPC.Stub.connect("localhost:#{port}") rect = Routeguide.Rectangle.new() - error = %GRPC.RPCError{message: "Deadline expired", status: 4} + error = %GRPC.RPCError{message: "Deadline expired", status: 4, details: []} assert {:error, ^error} = channel |> Routeguide.RouteGuide.Stub.list_features(rect, timeout: 500) diff --git a/test/grpc/integration/stub_test.exs b/test/grpc/integration/stub_test.exs index 7a256429..c9f3eb78 100644 --- a/test/grpc/integration/stub_test.exs +++ b/test/grpc/integration/stub_test.exs @@ -74,7 +74,8 @@ defmodule GRPC.Integration.StubTest do assert {:error, %GRPC.RPCError{ message: "Deadline expired", - status: GRPC.Status.deadline_exceeded() + status: GRPC.Status.deadline_exceeded(), + details: [] }} == channel |> Helloworld.Greeter.Stub.say_hello(req, timeout: 500) end) end