From f7904b117b59eaae7652b0fc31a3bb1ca939112d Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Fri, 3 May 2024 08:57:14 -0300 Subject: [PATCH 1/6] Translate errors from HTTP statuses This is inspired in how GRPC go approaches HTTP errors when performing GRPC requests. This provides meaningful errors in different situations. For example, AWS load balancer replies with a 504 when there are no live instances. There, a `unavailable` error is much more fitting than `internal`. --- lib/grpc/client/adapters/gun.ex | 12 ++---------- lib/grpc/rpc_error.ex | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/lib/grpc/client/adapters/gun.ex b/lib/grpc/client/adapters/gun.ex index 47f12843..c8bb4591 100644 --- a/lib/grpc/client/adapters/gun.ex +++ b/lib/grpc/client/adapters/gun.ex @@ -245,11 +245,7 @@ defmodule GRPC.Client.Adapters.Gun do )} end else - {:error, - GRPC.RPCError.exception( - GRPC.Status.internal(), - "status got is #{status} instead of 200" - )} + {:error, GRPC.RPCError.from_status(status)} end {:response, :nofin, status, headers} -> @@ -266,11 +262,7 @@ defmodule GRPC.Client.Adapters.Gun do {:response, headers, :nofin} end else - {:error, - GRPC.RPCError.exception( - GRPC.Status.internal(), - "status got is #{status} instead of 200" - )} + {:error, GRPC.RPCError.from_status(status)} end {:data, :fin, data} -> diff --git a/lib/grpc/rpc_error.ex b/lib/grpc/rpc_error.ex index 39ec37ed..d7760102 100644 --- a/lib/grpc/rpc_error.ex +++ b/lib/grpc/rpc_error.ex @@ -71,6 +71,23 @@ defmodule GRPC.RPCError do %{error | message: error.message || Status.status_message(error.status)} end + def from_status(401), do: exception(GRPC.Status.unauthorized(), status_message(401)) + def from_status(403), do: exception(GRPC.Status.permission_denied(), status_message(403)) + def from_status(404), do: exception(GRPC.Status.unimplemented(), status_message(404)) + def from_status(429), do: exception(GRPC.Status.unavailable(), status_message(429)) + + def from_status(status) when status in [502, 503, 504] do + exception(GRPC.Status.unavailable(), status_message(status)) + end + + def from_status(status) do + exception(GRPC.Status.internal(), status_message(status)) + end + + defp status_message(status) do + "got http status #{status_code} instead of 200" + end + defp parse_args([], acc), do: acc defp parse_args([{:status, status} | t], acc) when is_integer(status) do From ba0d3d30f30701e048bb76a45c3b9d13943ccba9 Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Fri, 3 May 2024 09:17:22 -0300 Subject: [PATCH 2/6] fixfix --- lib/grpc/rpc_error.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grpc/rpc_error.ex b/lib/grpc/rpc_error.ex index d7760102..24054590 100644 --- a/lib/grpc/rpc_error.ex +++ b/lib/grpc/rpc_error.ex @@ -85,7 +85,7 @@ defmodule GRPC.RPCError do end defp status_message(status) do - "got http status #{status_code} instead of 200" + "got http status #{status} instead of 200" end defp parse_args([], acc), do: acc From ed52bcea267c088bd47afbbc6ffebd75726820ce Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Fri, 3 May 2024 12:21:33 -0300 Subject: [PATCH 3/6] fix: unauthorized -> unauthenticated --- lib/grpc/rpc_error.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grpc/rpc_error.ex b/lib/grpc/rpc_error.ex index 24054590..b17950e5 100644 --- a/lib/grpc/rpc_error.ex +++ b/lib/grpc/rpc_error.ex @@ -71,7 +71,7 @@ defmodule GRPC.RPCError do %{error | message: error.message || Status.status_message(error.status)} end - def from_status(401), do: exception(GRPC.Status.unauthorized(), status_message(401)) + def from_status(401), do: exception(GRPC.Status.unauthenticated(), status_message(401)) def from_status(403), do: exception(GRPC.Status.permission_denied(), status_message(403)) def from_status(404), do: exception(GRPC.Status.unimplemented(), status_message(404)) def from_status(429), do: exception(GRPC.Status.unavailable(), status_message(429)) From 26fee70ae1ce7dbf2f9292088170c8896319a7a0 Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Sat, 4 May 2024 07:33:22 -0300 Subject: [PATCH 4/6] Add test --- test/grpc/integration/stub_test.exs | 24 +++++++++++++++++++++++ test/support/integration_test_case.ex | 28 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/test/grpc/integration/stub_test.exs b/test/grpc/integration/stub_test.exs index d24cdc68..08433efc 100644 --- a/test/grpc/integration/stub_test.exs +++ b/test/grpc/integration/stub_test.exs @@ -30,6 +30,30 @@ defmodule GRPC.Integration.StubTest do end) end + error_table = [ + {401, GRPC.Status.unauthenticated()}, + {403, GRPC.Status.permission_denied()}, + {404, GRPC.Status.unimplemented()}, + {429, GRPC.Status.unavailable()}, + {502, GRPC.Status.unavailable()}, + {503, GRPC.Status.unavailable()}, + {504, GRPC.Status.unavailable()}, + # General case + {518, GRPC.Status.internal()} + ] + + client_adapters = [GRPC.Client.Adapters.Gun, GRPC.Client.Adapters.Mint] + + for {http_code, expected_error_code} <- error_table, client_adapter <- client_adapters do + test "#{client_adapter} returns RPC Error when getting HTTP #{http_code}" do + run_error_server(unquote(http_code), fn port -> + {:ok, channel} = GRPC.Stub.connect("localhost:#{port}", adapter: unquote(client_adapter)) + req = %Helloworld.HelloRequest{name: "GRPC"} + {:error, %GRPC.RPCError{status: unquote(expected_error_code)}} = Helloworld.Greeter.Stub.say_hello(channel, req) + end) + end + end + test "you can disconnect stubs" do run_server(HelloServer, fn port -> {:ok, channel} = GRPC.Stub.connect("localhost:#{port}") diff --git a/test/support/integration_test_case.ex b/test/support/integration_test_case.ex index 4a4dbe9a..f16c5d85 100644 --- a/test/support/integration_test_case.ex +++ b/test/support/integration_test_case.ex @@ -9,6 +9,34 @@ defmodule GRPC.Integration.TestCase do end end + + defmodule ErrorHandler do + @behaviour :cowboy_handler + + @impl :cowboy_handler + def init(req, error_code) do + req = :cowboy_req.reply(error_code, %{"content-type" => "text/plain"}, "", req) + + {:ok, req, error_code} + end + end + + def run_error_server(error_code, func, port \\ 0) do + dispatch = :cowboy_router.compile([ + ({:_, [{:_, ErrorHandler, error_code}]}) + ]) + + {:ok, _} = :cowboy.start_clear("error_server", [port: port], %{env: %{dispatch: dispatch}}) + port = :ranch.get_port("error_server") + + try do + func.(port) + :ok + after + :ok = :cowboy.stop_listener("error_server") + end + end + def run_server(servers, func, port \\ 0, opts \\ []) do {:ok, _pid, port} = GRPC.Server.start(servers, port, opts) From cf8763c8b6c9d61766ce2fa8df8d6ef1b67fbdd0 Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Sat, 4 May 2024 07:35:35 -0300 Subject: [PATCH 5/6] s/from_status/from_http_status/ --- lib/grpc/client/adapters/gun.ex | 4 ++-- lib/grpc/rpc_error.ex | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/grpc/client/adapters/gun.ex b/lib/grpc/client/adapters/gun.ex index c8bb4591..5482a055 100644 --- a/lib/grpc/client/adapters/gun.ex +++ b/lib/grpc/client/adapters/gun.ex @@ -245,7 +245,7 @@ defmodule GRPC.Client.Adapters.Gun do )} end else - {:error, GRPC.RPCError.from_status(status)} + {:error, GRPC.RPCError.from_http_status(status)} end {:response, :nofin, status, headers} -> @@ -262,7 +262,7 @@ defmodule GRPC.Client.Adapters.Gun do {:response, headers, :nofin} end else - {:error, GRPC.RPCError.from_status(status)} + {:error, GRPC.RPCError.from_http_status(status)} end {:data, :fin, data} -> diff --git a/lib/grpc/rpc_error.ex b/lib/grpc/rpc_error.ex index b17950e5..e07714e1 100644 --- a/lib/grpc/rpc_error.ex +++ b/lib/grpc/rpc_error.ex @@ -71,17 +71,17 @@ defmodule GRPC.RPCError do %{error | message: error.message || Status.status_message(error.status)} end - def from_status(401), do: exception(GRPC.Status.unauthenticated(), status_message(401)) - def from_status(403), do: exception(GRPC.Status.permission_denied(), status_message(403)) - def from_status(404), do: exception(GRPC.Status.unimplemented(), status_message(404)) - def from_status(429), do: exception(GRPC.Status.unavailable(), status_message(429)) + def from_http_status(401), do: exception(Status.unauthenticated(), status_message(401)) + def from_http_status(403), do: exception(Status.permission_denied(), status_message(403)) + def from_http_status(404), do: exception(Status.unimplemented(), status_message(404)) + def from_http_status(429), do: exception(Status.unavailable(), status_message(429)) - def from_status(status) when status in [502, 503, 504] do - exception(GRPC.Status.unavailable(), status_message(status)) + def from_http_status(status) when status in [502, 503, 504] do + exception(Status.unavailable(), status_message(status)) end - def from_status(status) do - exception(GRPC.Status.internal(), status_message(status)) + def from_http_status(status) do + exception(Status.internal(), status_message(status)) end defp status_message(status) do From 0280a413220e3aef9daec04708220effad129499 Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Sat, 4 May 2024 08:54:18 -0300 Subject: [PATCH 6/6] format --- test/grpc/integration/stub_test.exs | 6 ++++-- test/support/integration_test_case.ex | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/test/grpc/integration/stub_test.exs b/test/grpc/integration/stub_test.exs index 08433efc..a628770a 100644 --- a/test/grpc/integration/stub_test.exs +++ b/test/grpc/integration/stub_test.exs @@ -44,12 +44,14 @@ defmodule GRPC.Integration.StubTest do client_adapters = [GRPC.Client.Adapters.Gun, GRPC.Client.Adapters.Mint] - for {http_code, expected_error_code} <- error_table, client_adapter <- client_adapters do + for {http_code, expected_error_code} <- error_table, client_adapter <- client_adapters do test "#{client_adapter} returns RPC Error when getting HTTP #{http_code}" do run_error_server(unquote(http_code), fn port -> {:ok, channel} = GRPC.Stub.connect("localhost:#{port}", adapter: unquote(client_adapter)) req = %Helloworld.HelloRequest{name: "GRPC"} - {:error, %GRPC.RPCError{status: unquote(expected_error_code)}} = Helloworld.Greeter.Stub.say_hello(channel, req) + + {:error, %GRPC.RPCError{status: unquote(expected_error_code)}} = + Helloworld.Greeter.Stub.say_hello(channel, req) end) end end diff --git a/test/support/integration_test_case.ex b/test/support/integration_test_case.ex index f16c5d85..91cb6925 100644 --- a/test/support/integration_test_case.ex +++ b/test/support/integration_test_case.ex @@ -9,7 +9,6 @@ defmodule GRPC.Integration.TestCase do end end - defmodule ErrorHandler do @behaviour :cowboy_handler @@ -22,9 +21,10 @@ defmodule GRPC.Integration.TestCase do end def run_error_server(error_code, func, port \\ 0) do - dispatch = :cowboy_router.compile([ - ({:_, [{:_, ErrorHandler, error_code}]}) - ]) + dispatch = + :cowboy_router.compile([ + {:_, [{:_, ErrorHandler, error_code}]} + ]) {:ok, _} = :cowboy.start_clear("error_server", [port: port], %{env: %{dispatch: dispatch}}) port = :ranch.get_port("error_server")