[thrift] handle timeouts and other errors gracefully (Erlang)

Reviewed By: iproctor

Test Plan: tutorial, channel

Revert Plan: ok

git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665190 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/lib/erl/lib/thrift/src/tErlProcessor.erl b/lib/erl/lib/thrift/src/tErlProcessor.erl
index ec263e3..a6d1073 100644 (file)
--- a/lib/erl/lib/thrift/src/tErlProcessor.erl
+++ b/lib/erl/lib/thrift/src/tErlProcessor.erl
@@ -60,4 +60,4 @@ process(This, Iprot, Oprot) ->
     GP      = oop:get(This, generatedProcessor),
     Handler = oop:get(This, handler),
 
-    apply(GP, process, [Handler, Iprot, Oprot]).
+    GP:process(Handler, Iprot, Oprot).

diff --git a/lib/erl/lib/thrift/src/thrift_logger.erl b/lib/erl/lib/thrift/src/thrift_logger.erl
index 286d40d..82ba772 100644 (file)
--- a/lib/erl/lib/thrift/src/thrift_logger.erl
+++ b/lib/erl/lib/thrift/src/thrift_logger.erl
@@ -138,7 +138,8 @@ handle_event1({What, _Gleader, {Pid, Format, Data}}, State) when is_list(Format)
            [Pid, LastMessage, Obj, Reason] = Data,
 
            %% TODO: move as much logic as possible out of thrift_logger
-           Ignore = error /= thrift_utils:unnest_record(Reason, tTransportException),
+           Ignore = (is_tuple(Reason) andalso size(Reason) >= 1 andalso element(1, Reason) == timeout)
+               orelse error /= thrift_utils:unnest_record(Reason, tTransportException),
 
            case Ignore of
                true ->
@@ -216,6 +217,9 @@ handle_thrift_info(req_processed, {Value}, State) ->
 handle_thrift_info(conn_accepted, {AddrString}, State) ->
     sformat("connection accepted from ~s", [AddrString]);
 
+handle_thrift_info(conn_timeout, {AddrString}, State) ->
+    sformat("connection timed out from ~s", [AddrString]);
+
 handle_thrift_info(conn_closed, {AddrString}, State) ->
     sformat("connection closed from ~s", [AddrString]);
 

diff --git a/lib/erl/lib/thrift/src/transport/tErlAcceptor.erl b/lib/erl/lib/thrift/src/transport/tErlAcceptor.erl
index 8093e00..f3308cf 100644 (file)
--- a/lib/erl/lib/thrift/src/transport/tErlAcceptor.erl
+++ b/lib/erl/lib/thrift/src/transport/tErlAcceptor.erl
@@ -89,10 +89,14 @@ accept(This, ListenSocket, GP, Handler) ->
            %% start_new(, ...)
            Processor = oop:start_new(tErlProcessor, [GP, Handler]), %% TODO
 
-           receive_loop(This, Processor, Prot, Prot),
-           
-           ?INFO(conn_closed, {AddrString}),
-
+           case receive_loop(This, Processor, Prot, Prot) of
+               conn_timeout ->
+                   ?INFO(conn_timeout, {AddrString});
+               conn_closed ->
+                   ?INFO(conn_closed, {AddrString});
+               {Class, Else} ->
+                   ?ERROR("unhandled ~p in tErlAcceptor: ~p", [Class, Else])
+           end,
            exit(normal);
 
        Else ->
@@ -110,15 +114,19 @@ receive_loop(This, Processor, Iprot, Oprot) ->
            ?INFO(req_processed, {Value}),
            receive_loop(This, Processor, Iprot, Oprot)
     catch
-       %% the following clause must be last because we might reexit
+       exit:{timeout, _} ->
+           conn_timeout;
+
+       %% the following clause must be last
+       %% cpiro: would be best to implement an is_a/2 guard BIF
        %% cpiro: breaks if it's a subclass of tTransportException
        %% since unnest_record knows nothing about oop
-       exit:Else ->
+       Class:Else ->
            case thrift_utils:unnest_record(Else, tTransportException) of
                {ok, TTE} when TTE#tTransportException.type == ?tTransportException_NOT_OPEN ->
-                   ok; %% will exit to tErlAcceptor
+                   conn_closed;
                _ ->
-                   exit(Else) %% shouldn't have caught it in the first place
+                   {Class, Else}
            end
     end.