Skip to content

Commit

Permalink
Add details field to gRPC error struct
Browse files Browse the repository at this point in the history
Adds a details field of type [any()] to the error struct and updates the
transport layer to properly encode + decode them.
  • Loading branch information
rishab-brex committed Oct 19, 2021
1 parent 2aff9a8 commit 57932f2
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 31 deletions.
34 changes: 25 additions & 9 deletions lib/grpc/adapter/cowboy/handler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand All @@ -280,25 +285,36 @@ 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}
end

# 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}
end

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}
Expand Down Expand Up @@ -428,7 +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)
Expand Down
12 changes: 10 additions & 2 deletions lib/grpc/adapter/gun.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
# TODO: stop assuming details is a list of strings only
headers["grpc-details"]
|> String.split(",")
|> Enum.filter(fn x -> x != "" end)
)}
end
else
Expand All @@ -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"],
# TODO: stop assuming details is a list of strings only
headers["grpc-details"]
|> String.split(",")
|> Enum.filter(fn x -> x != "" end)
)}
else
{:response, headers, :nofin}
Expand Down
31 changes: 20 additions & 11 deletions lib/grpc/error.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,30 @@ 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

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

Expand All @@ -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)"
Expand Down
11 changes: 10 additions & 1 deletion lib/grpc/stub.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
# TODO: stop assuming details is a list of strings only
details:
trailers["grpc-details"]
|> String.split(",")
|> Enum.filter(fn x -> x != "" end)
}}
end
end

Expand Down
8 changes: 5 additions & 3 deletions lib/grpc/transport/http2.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ 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
# TODO: stop assuming details is a list of String.t() only
%{
"grpc-status" => Integer.to_string(status),
"grpc-message" => message
"grpc-message" => message,
"grpc-details" => details |> Enum.join(",")
}
end

Expand Down
19 changes: 14 additions & 5 deletions test/grpc/integration/server_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ 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: ["hello", "world", 5]
end
end

Expand Down Expand Up @@ -101,6 +104,7 @@ defmodule GRPC.Integration.ServerTest do
end)
end

# THIS ONE
test "returns appropriate error for unary requests" do
run_server([HelloErrorServer], fn port ->
{:ok, channel} = GRPC.Stub.connect("localhost:#{port}")
Expand All @@ -109,7 +113,8 @@ defmodule GRPC.Integration.ServerTest do

assert %GRPC.RPCError{
status: GRPC.Status.unauthenticated(),
message: "Please authenticate"
message: "Please authenticate",
details: ["hello", "world", 5, 2]
} == reply
end)
end
Expand All @@ -120,7 +125,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
Expand All @@ -129,7 +138,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
Expand All @@ -148,7 +157,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)
Expand Down

0 comments on commit 57932f2

Please sign in to comment.