From 5330612c782dbd351e39dfe666fa90a8986a4ec5 Mon Sep 17 00:00:00 2001 From: alvarovillen Date: Thu, 15 Jun 2023 01:22:46 +0200 Subject: [PATCH 1/4] Handle not exception errors from the server --- lib/grpc/server/adapters/cowboy/handler.ex | 6 ++++ test/grpc/integration/server_test.exs | 40 ++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/lib/grpc/server/adapters/cowboy/handler.ex b/lib/grpc/server/adapters/cowboy/handler.ex index f0f5641b..c21a3479 100644 --- a/lib/grpc/server/adapters/cowboy/handler.ex +++ b/lib/grpc/server/adapters/cowboy/handler.ex @@ -376,6 +376,12 @@ defmodule GRPC.Server.Adapters.Cowboy.Handler do result = server.__call_rpc__(path, stream) case result do + {:ok, _stream, error = %GRPC.RPCError{message: nil, status: status}} -> + {:error, %{error | message: GRPC.Status.status_message(status)}} + + {:ok, _stream, error = %GRPC.RPCError{}} -> + {:error, error} + {:ok, stream, response} -> stream |> GRPC.Server.send_reply(response) diff --git a/test/grpc/integration/server_test.exs b/test/grpc/integration/server_test.exs index 378a6754..810cec85 100644 --- a/test/grpc/integration/server_test.exs +++ b/test/grpc/integration/server_test.exs @@ -60,6 +60,19 @@ defmodule GRPC.Integration.ServerTest do raise "unknown error(This is a test, please ignore it)" end + def say_hello(%{name: "handled error"}, _stream) do + %GRPC.RPCError{ + status: GRPC.Status.unauthenticated(), + message: "Please authenticate" + } + end + + def say_hello(%{name: "handled error without message"}, _stream) do + %GRPC.RPCError{ + status: GRPC.Status.unauthenticated() + } + end + def say_hello(_req, _stream) do raise GRPC.RPCError, status: GRPC.Status.unauthenticated(), message: "Please authenticate" end @@ -172,6 +185,33 @@ defmodule GRPC.Integration.ServerTest do end) end + test "return errors for handled errors" do + run_server([HelloErrorServer], fn port -> + {:ok, channel} = GRPC.Stub.connect("localhost:#{port}") + req = Helloworld.HelloRequest.new(name: "handled error") + {:error, reply} = channel |> Helloworld.Greeter.Stub.say_hello(req) + + assert %GRPC.RPCError{ + status: GRPC.Status.unauthenticated(), + message: "Please authenticate" + } == reply + end) + end + + test "return errors for handled errors with the default message of the status" do + run_server([HelloErrorServer], fn port -> + {:ok, channel} = GRPC.Stub.connect("localhost:#{port}") + req = Helloworld.HelloRequest.new(name: "handled error without message") + {:error, reply} = channel |> Helloworld.Greeter.Stub.say_hello(req) + + assert %GRPC.RPCError{ + status: GRPC.Status.unauthenticated(), + message: + "The request does not have valid authentication credentials for the operation" + } == reply + end) + end + test "returns appropriate error for stream requests" do run_server([FeatureErrorServer], fn port -> {:ok, channel} = GRPC.Stub.connect("localhost:#{port}") From 2c0c9263f5000f66f5da647a11c56f543d472670 Mon Sep 17 00:00:00 2001 From: alvarovillen Date: Thu, 15 Jun 2023 22:35:37 +0200 Subject: [PATCH 2/4] Handle errors returning error tuples --- lib/grpc/server.ex | 16 ++++++++++------ lib/grpc/server/adapters/cowboy/handler.ex | 13 +++++-------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/grpc/server.ex b/lib/grpc/server.ex index f1bacb68..0307e88a 100644 --- a/lib/grpc/server.ex +++ b/lib/grpc/server.ex @@ -184,12 +184,16 @@ defmodule GRPC.Server do ) do GRPC.Telemetry.server_span(server, endpoint, func_name, stream, fn -> last = fn r, s -> - reply = apply(server, func_name, [r, s]) - - if res_stream do - {:ok, stream} - else - {:ok, stream, reply} + case apply(server, func_name, [r, s]) do + %GRPC.RPCError{} = error -> + {:error, error} + + reply -> + if res_stream do + {:ok, stream} + else + {:ok, stream, reply} + end end end diff --git a/lib/grpc/server/adapters/cowboy/handler.ex b/lib/grpc/server/adapters/cowboy/handler.ex index c21a3479..0ebe1f0b 100644 --- a/lib/grpc/server/adapters/cowboy/handler.ex +++ b/lib/grpc/server/adapters/cowboy/handler.ex @@ -376,12 +376,6 @@ defmodule GRPC.Server.Adapters.Cowboy.Handler do result = server.__call_rpc__(path, stream) case result do - {:ok, _stream, error = %GRPC.RPCError{message: nil, status: status}} -> - {:error, %{error | message: GRPC.Status.status_message(status)}} - - {:ok, _stream, error = %GRPC.RPCError{}} -> - {:error, error} - {:ok, stream, response} -> stream |> GRPC.Server.send_reply(response) @@ -393,8 +387,11 @@ defmodule GRPC.Server.Adapters.Cowboy.Handler do GRPC.Server.send_trailers(stream, @default_trailers) {:ok, stream} - error -> - error + {:error, error = %GRPC.RPCError{message: nil, status: status}} -> + {:error, %{error | message: GRPC.Status.status_message(status)}} + + {:error, error = %GRPC.RPCError{}} -> + {:error, error} end end From 0a8a62a8e90ba01b9d8e1bf60a5d5925fa47b312 Mon Sep 17 00:00:00 2001 From: alvarovillen Date: Thu, 15 Jun 2023 22:59:40 +0200 Subject: [PATCH 3/4] Remove not necessary pattern-matching --- lib/grpc/server/adapters/cowboy/handler.ex | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/lib/grpc/server/adapters/cowboy/handler.ex b/lib/grpc/server/adapters/cowboy/handler.ex index 0ebe1f0b..7439ecae 100644 --- a/lib/grpc/server/adapters/cowboy/handler.ex +++ b/lib/grpc/server/adapters/cowboy/handler.ex @@ -347,11 +347,8 @@ defmodule GRPC.Server.Adapters.Cowboy.Handler do result = try do case do_call_rpc(server, path, stream) do - {:error, _} = err -> - err - - _ -> - :ok + {:error, _} = err -> err + _ -> :ok end catch kind, e -> @@ -361,14 +358,8 @@ defmodule GRPC.Server.Adapters.Cowboy.Handler do end case result do - {:error, %GRPC.RPCError{} = e} -> - exit({e, ""}) - - {:error, %{kind: kind}} -> - exit({:handle_error, kind}) - - other -> - other + {:error, %GRPC.RPCError{} = e} -> exit({e, ""}) + :ok -> :ok end end From e2100e58caf3faed74875936918b42f304db26fa Mon Sep 17 00:00:00 2001 From: alvarovillen Date: Thu, 15 Jun 2023 23:15:18 +0200 Subject: [PATCH 4/4] Define exit_handler as private --- lib/grpc/server/adapters/cowboy/handler.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grpc/server/adapters/cowboy/handler.ex b/lib/grpc/server/adapters/cowboy/handler.ex index 7439ecae..ce9c03ca 100644 --- a/lib/grpc/server/adapters/cowboy/handler.ex +++ b/lib/grpc/server/adapters/cowboy/handler.ex @@ -416,7 +416,7 @@ defmodule GRPC.Server.Adapters.Cowboy.Handler do :cowboy_req.reply(200, trailers, req) end - def exit_handler(pid, reason) do + defp exit_handler(pid, reason) do if Process.alive?(pid) do Process.exit(pid, reason) end