Stay connected after unknown_method errors

The unknown_method error was added in dc53fd9d, in part to support TTwitter
protocol negotion (i.e. Finagle). However it still closes the connection after
such an error, which means Finagle clients don't work. They need to be able to
call __can__finagle__trace__v3__, get a TApplicationException, then continue
calling other methods.

The code handling the :server_error tuple is what disconnects after an error.
In general disconnecting is the only safe choice given an internal server
error. However unknown_method belongs to a class of errors where it's safe to
remain connected -- the client has made an invalid request but otherwise the
connection is still in a valid state. So this diff adds a :client_error clause
that is like :server_error except that it keeps the connection open.

Tested with a Java client using Finagle. The client was able to call
__can__finagle__trace__v3__, get the error, then make a normal request.
This commit is contained in:
Preston Guillory 2021-07-11 15:30:41 -07:00
parent 3b6ef276e0
commit 13a1f3b3d5
3 changed files with 18 additions and 2 deletions

View File

@ -257,6 +257,19 @@ defmodule Thrift.Binary.Framed.ProtocolHandler do
send_reply([serialized_message | serialized_response], :continue, state) send_reply([serialized_message | serialized_response], :continue, state)
{:client_error, %TApplicationException{} = exc} ->
finish_span(span, result: "client_error")
serialized_message =
Binary.serialize(:message_begin, {:exception, sequence_id, method_name})
serialized_exception = Binary.serialize(:application_exception, exc)
response_size = IO.iodata_length(serialized_exception)
gauge(:response_size, response_size, state, method: method_name, result: "client_error")
send_reply([serialized_message | serialized_exception], :continue, state)
{:server_error, %TApplicationException{} = exc} -> {:server_error, %TApplicationException{} = exc} ->
finish_span(span, result: "error") finish_span(span, result: "error")

View File

@ -36,7 +36,7 @@ defmodule Thrift.Generator.Binary.Framed.Server do
message: "Unknown method: #{method}" message: "Unknown method: #{method}"
) )
{:server_error, error} {:client_error, error}
end end
end end
end end

View File

@ -446,7 +446,7 @@ defmodule Thrift.Generator.ServiceTest do
{:ok, nil} = Client.do_some_work(client, "work!") {:ok, nil} = Client.do_some_work(client, "work!")
end end
thrift_test "it handles unknown method calls", ctx do thrift_test "it stays connected after unknown method calls", ctx do
{:ok, client} = WrongClient.start_link("127.0.0.1", ctx.port) {:ok, client} = WrongClient.start_link("127.0.0.1", ctx.port)
# We have a client that implements different methods than the server to # We have a client that implements different methods than the server to
@ -458,5 +458,8 @@ defmodule Thrift.Generator.ServiceTest do
type: :unknown_method, type: :unknown_method,
message: "Unknown method: plang" message: "Unknown method: plang"
} }
# Still connected.
assert {:error, {:exception, exception}} = WrongClient.plang(client)
end end
end end