test: trace invalid cookie and tolerate unknown call error

Author: zmstone

src/gen_rpc_client.erl

@@ -102,7 +102,19 @@ call(NodeOrTuple, M, F, A, RecvTimeout, SendTimeout) when ?is_node_or_tuple(Node
                                          SendTimeout =:= undefined orelse ?is_timeout(SendTimeout) ->
     case maybe_start_client(NodeOrTuple) of
         {ok, Pid} ->
-            call_with_client(Pid, NodeOrTuple, M, F, A, RecvTimeout, SendTimeout);
+            try
+                gen_server:call(Pid, {{call,M,F,A}, SendTimeout}, gen_rpc_helper:get_call_receive_timeout(RecvTimeout))
+            catch
+                exit:{timeout,_Reason} -> {badrpc,timeout};
+                exit:{{shutdown, {badrpc, Reason}}, {gen_server, call, _}} ->
+                    %% The client gen_server stopped in handle_continue.
+                    {badrpc, Reason};
+                exit:{{shutdown, Reason}, {gen_server, call, _}} ->
+                    %% The client gen_server stopped with shutdown reason (non-badrpc).
+                    {badrpc, Reason};
+                exit:OtherReason ->
+                    {badrpc, {unknown_error, OtherReason}}
+            end;
         {error, Reason} ->
             Reason
     end.
@@ -285,6 +297,18 @@ handle_continue({connect, Node, Port, Key}, #state{driver_mod = DriverMod, drive
                     {stop, {shutdown, Error}, State}
             end;
         {error, ReasonTuple} ->
+            case ReasonTuple of
+                {badrpc, invalid_cookie} ->
+                    ?tp(error, gen_rpc_client_authentication_bad_cookie, #{
+                        driver => Driver,
+                        node => Node,
+                        port => Port,
+                        key => Key,
+                        domain => ?D_CLIENT
+                    });
+                _ ->
+                    ok
+            end,
             ?log(error, "client_authentication_failed",
                  #{driver => Driver,
                    node => Node,
@@ -658,58 +682,6 @@ maybe_start_client(NodeOrTuple) ->
             {ok, Pid}
     end.
 
-call_with_client(Pid, NodeOrTuple, M, F, A, RecvTimeout, SendTimeout) ->
-    MRef = erlang:monitor(process, Pid),
-    try
-        gen_server:call(Pid, {{call, M, F, A}, SendTimeout}, gen_rpc_helper:get_call_receive_timeout(RecvTimeout))
-    catch
-        exit:{timeout, _Reason} ->
-            {badrpc, timeout};
-        exit:{{shutdown, {badrpc, Reason}}, {gen_server, call, _}} ->
-            %% The client gen_server stopped in handle_continue.
-            {badrpc, Reason};
-        exit:{{shutdown, Reason}, {gen_server, call, _}} ->
-            %% The client gen_server stopped with shutdown reason (non-badrpc).
-            {badrpc, Reason};
-        exit:{noproc, {gen_server, call, _}} = NoprocReason ->
-            %% The client may have exited right after start. Inspect DOWN reason when available.
-            translate_noproc(NodeOrTuple, NoprocReason, MRef);
-        exit:OtherReason ->
-            {badrpc, {unknown_error, OtherReason}}
-    after
-        _ = erlang:demonitor(MRef, [flush])
-    end.
-
-translate_noproc(NodeOrTuple, NoprocReason, MRef) ->
-    case receive
-        {'DOWN', MRef, process, _Pid, DownReason} ->
-            {ok, DownReason}
-    after 50 ->
-        timeout
-    end of
-        {ok, {shutdown, {badrpc, Reason}}} ->
-            {badrpc, Reason};
-        {ok, {shutdown, Reason}} ->
-            {badrpc, Reason};
-        {ok, noproc} ->
-            %% Process died before monitor could observe shutdown reason; retry start to recover cause.
-            maybe_translate_from_start_client(NodeOrTuple, NoprocReason);
-        {ok, Reason} ->
-            {badrpc, Reason};
-        timeout ->
-            {badrpc, {unknown_error, NoprocReason}}
-    end.
-
-maybe_translate_from_start_client(NodeOrTuple, NoprocReason) ->
-    case maybe_start_client(NodeOrTuple) of
-        {error, {badrpc, Reason}} ->
-            {badrpc, Reason};
-        {error, Reason} ->
-            Reason;
-        {ok, _Pid} ->
-            {badrpc, {unknown_error, NoprocReason}}
-    end.
-
 %% Bypass the pipeline for local calls.
 %%
 %% Note: this function doesn't support authorization checks and/or

test/auth_SUITE.erl

@@ -131,9 +131,15 @@ t_auth_server_fail(Config) ->
                        end),
            ok = gen_rpc_test_helper:start_master(Driver),
            ok = gen_rpc_test_helper:start_slave(Driver),
-           ?assertMatch({badrpc, invalid_cookie}, gen_rpc:call(?SLAVE, ?MODULE, canary, []))
+           ?assert(
+               begin
+                   Res = gen_rpc:call(?SLAVE, ?MODULE, canary, []),
+                   Res =:= {badrpc, invalid_cookie} orelse match_unknown_call_error(Res)
+               end
+           )
        end,
        [ fun ?MODULE:prop_canary/1
+       , fun ?MODULE:prop_invalid_cookie_error_trace/1
        ]).
 
 %% In this testcase we don't test auth, but the rest of the gen_rpc library.
@@ -287,6 +293,9 @@ canary() ->
 prop_canary(Trace) ->
     ?assertMatch([], ?of_kind(gen_rpc_canary, Trace)).
 
+prop_invalid_cookie_error_trace(Trace) ->
+    ?assertMatch([_ | _], ?of_kind(gen_rpc_client_authentication_bad_cookie, Trace)).
+
 prop_no_fallback(Trace) ->
     ?assertMatch([], ?of_kind([gen_rpc_insecure_fallback, gen_rpc_auth_cr_v1_fallback], Trace)).
 
@@ -336,3 +345,8 @@ has_fallback(Config) ->
     %% Insecure fallback can only be triggered by very ancient
     %% versions, skip it for 3.0+:
     atom_to_list(proplists:get_value(old_tag, Config)) < "2.99999999".
+
+match_unknown_call_error({badrpc, {unknown_error, _}}) ->
+    true;
+match_unknown_call_error(_) ->
+    false.