Exceptions Bypass Interceptors

[asummers]

Describe the bug The type spec for the Server Interceptor next return type is:

  @type rpc_return :: {:ok, Stream.t(), struct} | {:ok, Stream.t()} | {:error, GRPC.RPCError.t()}

but when raising an error, e.g

 raise GRPC.RPCError, status: :unauthenticated, message: "Please authenticate"

the interceptor seems to be bypassed entirely and is never given the {:error, exception} tuple .

To Reproduce Create an interceptor with the following call/2:

  def call(request, stream, next, _options) do
    IO.inspect "before"
    result = next.(request, stream)
    IO.inspect "after"
    result
  end

create a protbuf call that does something like the following:

  def get(_request, _stream) do
    raise GRPC.RPCError, status: :unauthenticated, message: "Please authenticate"
  end

Make a call with e.g. grpcurl and notice that it responds appropriately:

ERROR:
  Code: Unauthenticated
  Message: Please authenticate

But then notice the logs from the app only contain:

iex(2)> "before"

Expected behavior Exceptions are turned into errors centrally so that interceptors can avoid having to rescue, or requiring a "rescue interceptor" that does this for you. Things like the included Logger interceptor do not work appropriately without doing something like this.

Versions:

• OS: OSX
• Erlang: 22.3.4.1
• Elixir: 1.10.2
• mix.lock(grpc, gun, cowboy, cowlib):

  "grpc": {:git, "https://github.com/elixir-grpc/grpc.git", "d8a58cca77a5fc106669f8ddaee634e0a3ba9267", []},
  "gun": {:hex, :grpc_gun, "2.0.0", "f99678a2ab975e74372a756c86ec30a8384d3ac8a8b86c7ed6243ef4e61d2729", [:rebar3], [{:cowlib, "~> 2.8.0", [hex: :cowlib, repo: "hexpm", optional: false]}], "hexpm", "03dbbca1a9c604a0267a40ea1d69986225091acb822de0b2dbea21d5815e410b"},
  "cowboy": {:hex, :cowboy, "2.7.0", "91ed100138a764355f43316b1d23d7ff6bdb0de4ea618cb5d8677c93a7a2f115", [:rebar3], [{:cowlib, "~> 2.8.0", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "~> 1.7.1", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "04fd8c6a39edc6aaa9c26123009200fc61f92a3a94f3178c527b70b767c6e605"},
  "cowlib": {:hex, :cowlib, "2.9.0", 
"8365736a2ada74d5e8640c9b03efff15aceffcf2c7cba2e5ffd0c549f54bf0da", [:rebar3], [], "hexpm", "e8a93bbdf5c4f3d63fbb0cae422de2b58227955bebad5fed978f7a83b0ca4c89"},

[asummers]

Note that adding an interceptor with:

  @impl GRPC.ServerInterceptor
  @spec call(
          GRPC.Server.rpc_req(),
          GRPC.Server.Stream.t(),
          GRPC.ServerInterceptor.next(),
          GRPC.ServerInterceptor.options()
        ) :: GRPC.ServerInterceptor.rpc_return()
  def call(request, stream, next, _options) do
    try do
      next.(request, stream)
    rescue
      exception in GRPC.RPCError ->
        {:error, exception}

      exception ->
        reraise exception,  __STACKTRACE__
    end
  end

into the interceptor list at the bottom restores what I believe to be the intended behavior, but this should not be necessary.

[tony612]

At first, exception is catched in the innermost layer. Now it's changed https://github.com/elixir-grpc/grpc/pull/96. The main reason is now we can support catching the exceptions in interceptors, which is more flexible and exceptions in interceptors are also handled. But yes, as you said, now interceptors may need to handle exceptions. I'm thinking maybe we can improve this by:

1. We only rescue GRPC.RPCError in the innermost layer.
2. We provide an interceptor, which handle errors like what we did before. You can choose to include it or not.
3. Like 2, but we can set this interceptor to default and provide a option to remove it if we want to handle exceptions in custom interceptors.

[tony612]

btw, I checked Plug code. It seems it also doesn't catch the exceptions in the innermost layer.

[hkrutzer]

I think Plug has https://hexdocs.pm/plug/Plug.ErrorHandler.html. Your option 2 seems best.

[polvalente]

@asummers @hkrutzer is this issue still relevant?