using #amqp_error{} instead of {amqp, ...}

Author: Vlad Ionescu

include/rabbit.hrl

@@ -67,6 +67,8 @@
 -record(ssl_socket, {tcp, ssl}).
 -record(delivery, {mandatory, immediate, txn, sender, message}).
 
+-record(amqp_error, {name, expl = "", method = none}).
+
 %%----------------------------------------------------------------------------
 
 -ifdef(use_specs).
@@ -154,7 +156,10 @@
                 port     :: non_neg_integer()}).
 -type(not_found() :: {'error', 'not_found'}).
 -type(routing_result() :: 'routed' | 'unroutable' | 'not_delivered').
-
+-type(amqp_error() ::
+      #amqp_error{name   :: atom(),
+                  expl   :: string(),
+                  method :: atom()}).
 -endif.
 
 %%----------------------------------------------------------------------------

include/rabbit_framing_spec.hrl

@@ -50,8 +50,6 @@
 %% TODO: make this more precise
 -type(amqp_method_name() :: atom()).
 -type(channel_number() :: non_neg_integer()).
-%% TODO: make this more precise
--type(amqp_error() :: {bool(), non_neg_integer(), binary()}).
 -type(resource_name() :: binary()).
 -type(routing_key() :: binary()).
 -type(username() :: binary()).

src/rabbit_channel.erl

@@ -125,11 +125,12 @@ handle_cast({method, Method, Content}, State) ->
         stop ->
             {stop, normal, State#ch{state = terminating}}
     catch
-        exit:{amqp, Error, Explanation, none} ->
+        exit:Reason = #amqp_error{} ->
             ok = rollback_and_notify(State),
-            Reason = {amqp, Error, Explanation,
-                      rabbit_misc:method_record_type(Method)},
-            State#ch.reader_pid ! {channel_exit, State#ch.channel, Reason},
+            CompleteReason = Reason#amqp_error{method =
+                rabbit_misc:method_record_type(Method)},
+            State#ch.reader_pid !
+                {channel_exit, State#ch.channel, CompleteReason},
             {stop, normal, State#ch{state = terminating}};
         exit:normal ->
             {stop, normal, State};

src/rabbit_misc.erl

@@ -35,7 +35,7 @@
 -include_lib("kernel/include/file.hrl").
 
 -export([method_record_type/1, polite_pause/0, polite_pause/1]).
--export([die/1, frame_error/2, protocol_error/3, protocol_error/4]).
+-export([die/1, frame_error/2, amqp_error/4, protocol_error/3, protocol_error/4]).
 -export([not_found/1]).
 -export([get_config/1, get_config/2, set_config/2]).
 -export([dirty_read/1]).
@@ -74,10 +74,9 @@
 -spec(polite_pause/1 :: (non_neg_integer()) -> 'done').
 -spec(die/1 :: (atom()) -> no_return()).
 -spec(frame_error/2 :: (atom(), binary()) -> no_return()).
--spec(protocol_error/3 ::
-      (atom() | amqp_error(), string(), [any()]) -> no_return()).
--spec(protocol_error/4 ::
-      (atom() | amqp_error(), string(), [any()], atom()) -> no_return()).
+-spec(amqp_error/4 :: (atom(), string(), [any()], atom()) -> amqp_error()).
+-spec(protocol_error/3 :: (atom(), string(), [any()]) -> no_return()).
+-spec(protocol_error/4 :: (atom(), string(), [any()], atom()) -> no_return()).
 -spec(not_found/1 :: (r(atom())) -> no_return()).
 -spec(get_config/1 :: (atom()) -> {'ok', any()} | not_found()).
 -spec(get_config/2 :: (atom(), A) -> A).
@@ -144,15 +143,17 @@ die(Error) ->
     protocol_error(Error, "~w", [Error]).
 
 frame_error(MethodName, BinaryFields) ->
-    protocol_error(frame_error, "cannot decode ~w",
-                   [BinaryFields], MethodName).
+    protocol_error(frame_error, "cannot decode ~w", [BinaryFields], MethodName).
 
-protocol_error(Error, Explanation, Params) ->
-    protocol_error(Error, Explanation, Params, none).
+amqp_error(Name, Explanation, Params, Method) ->
+    #amqp_error{name = Name,
+                expl = lists:flatten(io_lib:format(Explanation, Params)),
+                method = Method}.
 
-protocol_error(Error, Explanation, Params, Method) ->
-    CompleteExplanation = lists:flatten(io_lib:format(Explanation, Params)),
-    exit({amqp, Error, CompleteExplanation, Method}).
+protocol_error(Name, Explanation, Params) ->
+    protocol_error(Name, Explanation, Params, none).
+protocol_error(Name, Explanation, Params, Method) ->
+    exit(amqp_error(Name, Explanation, Params, Method)).
 
 not_found(R) -> protocol_error(not_found, "no ~s", [rs(R)]).
 

src/rabbit_reader.erl

@@ -269,12 +269,10 @@ mainloop(Parent, Deb, State = #v1{sock= Sock, recv_ref = Ref}) ->
             throw({inet_error, Reason});
         {'EXIT', Parent, Reason} ->
             if State#v1.connection_state =:= running ->
-                    send_exception(
-                      State, 0,
-                      {amqp, connection_forced,
-                       io_lib:format(
-                         "broker forced connection closure with reason '~w'",
-                         [Reason]), none});
+                    send_exception(State, 0,
+                        rabbit_misc:amqp_error(connection_forced,
+                            "broker forced connection closure with reason '~w'",
+                            [Reason], none));
                true -> ok
             end,
             %% this is what we are expected to do according to 
@@ -567,12 +565,13 @@ handle_method0(MethodName, FieldsBin, State) ->
                          MethodName, FieldsBin),
                        State)
     catch exit:Reason ->
-            CompleteReason =
-                case Reason of
-                    {amqp, Error, Explanation, none} ->
-                        {amqp, Error, Explanation, MethodName};
-                    OtherReason -> OtherReason
-                end,
+        CompleteReason =
+            case Reason of
+                #amqp_error{method = none} ->
+                    Reason#amqp_error{method = MethodName};
+                _ ->
+                    Reason
+            end,
             case State#v1.connection_state of
                 running -> send_exception(State, 0, CompleteReason);
                 Other   -> throw({channel0_error, Other, CompleteReason})
@@ -793,18 +792,21 @@ map_exception(Channel, Reason) ->
         end,
     {ShouldClose, CloseChannel, CloseMethod}.
 
-lookup_amqp_exception({amqp, {ShouldClose, Code, Text}, Expl, Method}) ->
+lookup_amqp_exception(#amqp_error{name = Name,
+                                  expl = Expl,
+                                  method = Method}) ->
+    {ShouldClose, Code, Text} = rabbit_framing:lookup_amqp_exception(Name),
     ExplBin = list_to_binary(Expl),
     CompleteTextBin = <<Text/binary, " - ", ExplBin/binary>>,
-    SafeTextBin = if size(CompleteTextBin) > 255 ->
-                          <<CompleteTextBin:252/binary, "...">>;
-                     true ->
-                          CompleteTextBin
-                  end,
-    {ShouldClose, Code, SafeTextBin, Method};
-lookup_amqp_exception({amqp, ErrorName, Expl, Method}) ->
-    Details = rabbit_framing:lookup_amqp_exception(ErrorName), 
-    lookup_amqp_exception({amqp, Details, Expl, Method});
+    SafeTextBin =
+        if
+            size(CompleteTextBin) > 255 ->
+                <<CompleteTextBin:252/binary, "...">>;
+            true ->
+                CompleteTextBin
+       end,
+   {ShouldClose, Code, SafeTextBin, Method};
+
 lookup_amqp_exception(Other) ->
     rabbit_log:warning("Non-AMQP exit reason '~p'~n", [Other]),
-    {true, ?INTERNAL_ERROR, <<"INTERNAL_ERROR">>, none}.
+    lookup_amqp_exception(#amqp_error{name = internal_error}).