Merge pull request #42 from mitchellwrosen/master

websocket_terminate type overhaul

Author: jeremyong

src/websocket_client.erl

@@ -2,8 +2,7 @@
 %% @doc Erlang websocket client
 -module(websocket_client).
 
--export([
-         start_link/3,
+-export([start_link/3,
          start_link/4,
          cast/2,
          send/2
@@ -182,7 +181,7 @@ handle_websocket_message(WSReq, HandlerState, Buffer, Message) ->
             ok = send(Frame, WSReq),
             websocket_loop(WSReq, HandlerState, Buffer);
         {_Closed, Socket} ->
-            websocket_close(WSReq, HandlerState, {remote, closed});
+            websocket_close(WSReq, HandlerState, remote);
         {_TransportType, Socket, Data} ->
             case Remaining of
                 undefined ->
@@ -194,18 +193,11 @@ handle_websocket_message(WSReq, HandlerState, Buffer, Message) ->
             end;
         Msg ->
             try Handler:websocket_info(Msg, WSReq, HandlerState) of
-              HandlerResponse ->
-                handle_response(WSReq, HandlerResponse, Buffer)
-            catch Class:Reason ->
-              error_logger:error_msg(
-                "** Websocket client ~p terminating in ~p/~p~n"
-                "   for the reason ~p:~p~n"
-                "** Last message was ~p~n"
-                "** Handler state was ~p~n"
-                "** Stacktrace: ~p~n~n",
-                [Handler, websocket_info, 3, Class, Reason, Msg, HandlerState,
-                  erlang:get_stacktrace()]),
-              websocket_close(WSReq, HandlerState, Reason)
+                HandlerResponse ->
+                    handle_response(WSReq, HandlerResponse, Buffer)
+            catch
+                _:Reason ->
+                    websocket_close(WSReq, HandlerState, {handler, Reason})
             end
     end.
 
@@ -224,16 +216,26 @@ cancel_keepalive_timer(WSReq) ->
                       Reason :: tuple()) -> ok.
 websocket_close(WSReq, HandlerState, Reason) ->
     Handler = websocket_req:handler(WSReq),
-    try Handler:websocket_terminate(Reason, WSReq, HandlerState)
-    catch Class:Reason2 ->
-      error_logger:error_msg(
-        "** Websocket handler ~p terminating in ~p/~p~n"
-        "   for the reason ~p:~p~n"
+    try Handler:websocket_terminate(Reason, WSReq, HandlerState) of
+        _ ->
+            case Reason of
+                normal -> ok;
+                _      -> error_info(Handler, Reason, HandlerState)
+            end,
+            exit(Reason)
+    catch
+        _:Reason2 ->
+            error_info(Handler, Reason2, HandlerState),
+            exit(Reason2)
+    end.
+
+error_info(Handler, Reason, State) ->
+    error_logger:error_msg(
+        "** Websocket handler ~p terminating~n"
+        "** for the reason ~p~n"
         "** Handler state was ~p~n"
         "** Stacktrace: ~p~n~n",
-        [Handler, websocket_terminate, 3, Class, Reason2, HandlerState,
-          erlang:get_stacktrace()])
-    end.
+        [Handler, Reason, State, erlang:get_stacktrace()]).
 
 %% @doc Key sent in initial handshake
 -spec generate_ws_key() ->
@@ -339,18 +341,24 @@ retrieve_frame(WSReq, HandlerState, Opcode, Len, Data, Buffer) ->
     end,
     case OpcodeName of
         close when byte_size(FullPayload) >= 2 ->
-            << CodeBin:2/binary, ClosePayload/binary >> = FullPayload,
+            << CodeBin:2/binary, _ClosePayload/binary >> = FullPayload,
             Code = binary:decode_unsigned(CodeBin),
             Reason = case Code of
-                         1000 -> {normal, ClosePayload};
-                         1002 -> {error, badframe, ClosePayload};
-                         1007 -> {error, badencoding, ClosePayload};
-                         1011 -> {error, handler, ClosePayload};
-                         _ -> {remote, Code, ClosePayload}
+                         % 1000 indicates a normal closure, meaning that the purpose for
+                         % which the connection was established has been fulfilled.
+                         1000 -> normal;
+
+                         % 1001 indicates that an endpoint is "going away", such as a server
+                         % going down or a browser having navigated away from a page.
+                         1001 -> normal;
+
+                         % See https://tools.ietf.org/html/rfc6455#section-7.4.1
+                         % for error code descriptions.
+                         _ -> {remote, Code}
                      end,
             websocket_close(WSReq, HandlerState, Reason);
         close ->
-            websocket_close(WSReq, HandlerState, {remote, <<>>});
+            websocket_close(WSReq, HandlerState, remote);
         %% Non-control continuation frame
         _ when Opcode < 8, Continuation =/= undefined, Fin == 0 ->
             %% Append to previously existing continuation payloads and continue
@@ -366,36 +374,21 @@ retrieve_frame(WSReq, HandlerState, Opcode, Len, Data, Buffer) ->
             try Handler:websocket_handle(
                                 {ContinuationOpcodeName, DefragPayload},
                                 WSReq2, HandlerState) of
-              HandlerResponse ->
-                handle_response(websocket_req:remaining(undefined, WSReq1),
-                                HandlerResponse, Rest)
-            catch Class:Reason ->
-              error_logger:error_msg(
-                "** Websocket client ~p terminating in ~p/~p~n"
-                "   for the reason ~p:~p~n"
-                "** Websocket message was ~p~n"
-                "** Handler state was ~p~n"
-                "** Stacktrace: ~p~n~n",
-                [Handler, websocket_handle, 3, Class, Reason, {ContinuationOpcodeName, DefragPayload}, HandlerState,
-                  erlang:get_stacktrace()]),
-              websocket_close(WSReq, HandlerState, Reason)
+                HandlerResponse ->
+                    handle_response(websocket_req:remaining(undefined, WSReq1),
+                                    HandlerResponse, Rest)
+            catch _:Reason ->
+                websocket_close(WSReq, HandlerState, {handler, Reason})
             end;
         _ ->
             try Handler:websocket_handle(
                                 {OpcodeName, FullPayload},
                                 WSReq, HandlerState) of
-              HandlerResponse ->
-                handle_response(websocket_req:remaining(undefined, WSReq),
-                                HandlerResponse, Rest)
-            catch Class:Reason ->
-              error_logger:error_msg(
-                "** Websocket client ~p terminating in ~p/~p~n"
-                "   for the reason ~p:~p~n"
-                "** Handler state was ~p~n"
-                "** Stacktrace: ~p~n~n",
-                [Handler, websocket_handle, 3, Class, Reason, HandlerState,
-                  erlang:get_stacktrace()]),
-              websocket_close(WSReq, HandlerState, Reason)
+                HandlerResponse ->
+                    handle_response(websocket_req:remaining(undefined, WSReq),
+                                    HandlerResponse, Rest)
+            catch _:Reason ->
+                websocket_close(WSReq, HandlerState, {handler, Reason})
             end
     end.
 
@@ -407,11 +400,14 @@ handle_response(WSReq, {reply, Frame, HandlerState}, Buffer) ->
            %% we can still have more messages in buffer
            case websocket_req:remaining(WSReq) of
                %% buffer should not contain uncomplete messages
-               undefined -> retrieve_frame(WSReq, HandlerState, Buffer);
+               undefined ->
+                   retrieve_frame(WSReq, HandlerState, Buffer);
                %% buffer contain uncomplete message that shouldnt be parsed
-               _ -> websocket_loop(WSReq, HandlerState, Buffer)
+               _ ->
+                   websocket_loop(WSReq, HandlerState, Buffer)
            end;
-        Reason -> websocket_close(WSReq, HandlerState, Reason)
+        {error, Reason} ->
+            websocket_close(WSReq, HandlerState, {local, Reason})
     end;
 handle_response(WSReq, {ok, HandlerState}, Buffer) ->
     %% we can still have more messages in buffer
@@ -424,7 +420,7 @@ handle_response(WSReq, {ok, HandlerState}, Buffer) ->
 
 handle_response(WSReq, {close, Payload, HandlerState}, _) ->
     send({close, Payload}, WSReq),
-    websocket_close(WSReq, HandlerState, {normal, Payload}).
+    websocket_close(WSReq, HandlerState, normal).
 
 %% @doc Encodes the data with a header (including a masking key) and
 %% masks the data

src/websocket_client_handler.erl

@@ -4,33 +4,26 @@
 -type keepalive() :: integer().
 
 -type close_reason() ::
-    % Either the local end was closed via a `{closed, Reason, State}` tuple
-    % returned from websocket_handle/3 or websocket_info/3, or the remote end
-    % was closed during graceful teardown of the connection via a close frame
-    % (see https://tools.ietf.org/html/rfc6455#section-5.5.1).
-    {normal, binary()} |
-    % The transport failed to send (see http://erlang.org/doc/man/gen_tcp.html#send-2
-    % or http://erlang.org/doc/man/ssl.html#send-2)
-    {error, term()} |
-    % The remote end was closed due to a protocol error
-    % (see https://tools.ietf.org/html/rfc6455#section-7.4.1, status code 1002).
-    {error, badframe, binary()} |
-    % The remote end was closed due to an encoding error
-    % (see https://tools.ietf.org/html/rfc6455#section-7.4.1, status code 1007).
-    {error, badencoding, binary()} |
-    % The remote end was closed unexpectedly
-    % (see https://tools.ietf.org/html/rfc6455#section-7.4.1, status code 1011).
-    {error, handler, binary()} |
-    % The remote host closed the socket.
-    {remote, closed} |
-    % The remote end was closed for some other reason
-    % (see https://tools.ietf.org/html/rfc6455#section-7.4.1).
-    {remote, integer(), binary()} |
-    % The remote host closed the socket.
-    % TODO: How is this different than `{remote, closed}`?
-    {remote, binary()} |
-    % The handler terminated unexpectedly in websocket_handle/3 or websocket_info/3.
-    term().
+    % Either:
+    % - The websocket was closed by a handler via a `{closed, Reason, State}` tuple
+    % returned from websocket_handle/3 or websocket_info/3.
+    % - A 'close' frame was received with code 1000 or 1001.
+    normal |
+    % The local end failed to send (see http://erlang.org/doc/man/gen_tcp.html#send-2
+    % or http://erlang.org/doc/man/ssl.html#send-2). The second element in the
+    % tuple is the same term that was wrapped in an `{error, Reason}` tuple by
+    % `send/2`, i.e. `{error, closed}` will become `{local, closed}`, and not
+    % `{local, {error, closed}}`.
+    {local, term()} |
+    % The remote end either closed abruptly, or closed after sending a 'close' frame
+    % without a status code.
+    remote |
+    % The remote end closed with a status code (see https://tools.ietf.org/html/rfc6455#section-7.4.1).
+    {remote, integer()} |
+    % An asynchronous exception was raised during message handling, either in
+    % websocket_handle/3 or websocket_info/3. The term raised is passed as the
+    % second element in this tuple.
+    {handler, term()}.
 
 -callback init(list(), websocket_req:req()) ->
     {ok, state()}
@@ -44,6 +37,6 @@
 -callback websocket_info(any(), websocket_req:req(), state()) ->
     {ok, state()}
         | {reply, websocket_req:frame(), state()}
-        | {close, binary(),  state()}.
+        | {close, binary(), state()}.
 
 -callback websocket_terminate(close_reason(), websocket_req:req(), state()) -> ok.