TD-431: Return timeout error upon negative timeout leftover after end… (#29)

* TD-431: Return timeout error upon negative timeout leftover after endpoint resolution in document request API

* Adds testcase endpoint_resolve_timeout_means_unavailable with mocked resolve_endpoint/2 to bouncer_tests_SUITE

rebar.config

@@ -127,7 +127,9 @@
 
     {test, [
         {cover_enabled, true},
-        {deps, []},
+        {deps, [
-        {dialyzer, [{plt_extra_apps, [eunit, common_test, gun]}]}
+            {meck, "0.9.2"}
+        ]},
+        {dialyzer, [{plt_extra_apps, [eunit, common_test, gun, meck]}]}
     ]}
 ]}.

src/bouncer_opa_client.erl

@@ -86,20 +86,7 @@ request_document(RulesetID, Input, Client) ->
     try
         ResolvedEndpoint = resolve_endpoint(Endpoint, RequestTimeout),
         TimeoutLeft = Deadline - erlang:monotonic_time(millisecond),
-        GunnerOpts = make_gunner_opts(TimeoutLeft, Client),
+        do_request_document(TimeoutLeft, ResolvedEndpoint, Path, Body, Headers, Client)
-        %% Trying the synchronous API first
-        case gunner:post(?GUNNER_POOL_ID, ResolvedEndpoint, Path, Body, Headers, GunnerOpts) of
-            {ok, 200, _, Response} when is_binary(Response) ->
-                decode_document(Response);
-            {ok, 404, _, _} ->
-                {error, notfound};
-            {ok, Code, _, Response} ->
-                {error, {unknown, {Code, Response}}};
-            {error, {unknown, Reason}} ->
-                {error, {unknown, Reason}};
-            {error, Reason} ->
-                {error, {unavailable, Reason}}
-        end
     catch
         throw:{resolve_failed, ResolvError} ->
             {error, {unavailable, ResolvError}}
@@ -117,6 +104,25 @@ decode_document(Response) ->
     end.
 
 %%
+do_request_document(TimeoutLeft, _ResolvedEndpoint, _Path, _Body, _Headers, _Client) when
+    TimeoutLeft =< 0
+->
+    {error, {unavailable, timeout}};
+do_request_document(TimeoutLeft, ResolvedEndpoint, Path, Body, Headers, Client) ->
+    GunnerOpts = make_gunner_opts(TimeoutLeft, Client),
+    %% Trying the synchronous API first
+    case gunner:post(?GUNNER_POOL_ID, ResolvedEndpoint, Path, Body, Headers, GunnerOpts) of
+        {ok, 200, _, Response} when is_binary(Response) ->
+            decode_document(Response);
+        {ok, 404, _, _} ->
+            {error, notfound};
+        {ok, Code, _, Response} ->
+            {error, {unknown, {Code, Response}}};
+        {error, {unknown, Reason}} ->
+            {error, {unknown, Reason}};
+        {error, Reason} ->
+            {error, {unavailable, Reason}}
+    end.
 
 resolve_endpoint({{resolve, dns, Hostname, Opts}, Port}, Timeout) ->
     case gunner_resolver:resolve_endpoint({Hostname, Port}, make_resolver_opts(Timeout, Opts)) of

test/bouncer_tests_SUITE.erl

@@ -28,6 +28,7 @@
 -export([connect_failed_means_unavailable/1]).
 -export([connect_timeout_means_unavailable/1]).
 -export([request_timeout_means_unknown/1]).
+-export([endpoint_resolve_timeout_means_unavailable/1]).
 
 -behaviour(bouncer_arbiter_pulse).
 
@@ -79,7 +80,8 @@ groups() ->
         {network_error_mapping, [], [
             connect_failed_means_unavailable,
             connect_timeout_means_unavailable,
-            request_timeout_means_unknown
+            request_timeout_means_unknown,
+            endpoint_resolve_timeout_means_unavailable
         ]}
     ].
 
@@ -158,10 +160,16 @@ stop_bouncer(C) ->
     ).
 
 -spec init_per_testcase(atom(), config()) -> config().
+init_per_testcase(endpoint_resolve_timeout_means_unavailable = Name, C) ->
+    meck:new(gunner_resolver, [no_link, passthrough]),
+    [{testcase, Name} | C];
 init_per_testcase(Name, C) ->
     [{testcase, Name} | C].
 
 -spec end_per_testcase(atom(), config()) -> ok.
+end_per_testcase(endpoint_resolve_timeout_means_unavailable, _C) ->
+    meck:unload(gunner_resolver),
+    ok;
 end_per_testcase(_Name, _C) ->
     ok.
 
@@ -508,6 +516,7 @@ format_ts(Ts, Unit) ->
 -spec connect_failed_means_unavailable(config()) -> ok.
 -spec connect_timeout_means_unavailable(config()) -> ok.
 -spec request_timeout_means_unknown(config()) -> ok.
+-spec endpoint_resolve_timeout_means_unavailable(config()) -> ok.
 
 connect_failed_means_unavailable(C) ->
     C1 = start_bouncer(
@@ -586,6 +595,45 @@ request_timeout_means_unknown(C) ->
         stop_bouncer(C1)
     end.
 
+endpoint_resolve_timeout_means_unavailable(C) ->
+    ok = meck:expect(gunner_resolver, resolve_endpoint, fun(Endpoint, Opts) ->
+        Timeout = maps:get(timeout, Opts),
+        timer:sleep(Timeout + 10),
+        meck:passthrough([Endpoint, Opts])
+    end),
+    %% Don't need actual OPA
+    C1 = start_bouncer(
+        [
+            {opa, #{
+                %% But we need an endpoint resolution try
+                endpoint => ?OPA_ENDPOINT_RESOLVE,
+                pool_opts => #{
+                    connection_opts => #{
+                        transport => tcp,
+                        event_handler => {ct_gun_event_h, []}
+                    }
+                }
+            }}
+        ],
+        C
+    ),
+    Client = mk_client(C1),
+    try
+        ?assertError(
+            {woody_error, {external, resource_unavailable, <<"timeout">>}},
+            call_judge(?API_RULESET_ID, ?CONTEXT(#{}), Client)
+        ),
+        ?assertMatch(
+            [
+                {judgement, started},
+                {judgement, {failed, {unavailable, timeout}}}
+            ],
+            flush_beats(Client, C1)
+        )
+    after
+        stop_bouncer(C1)
+    end.
+
 start_proxy_bouncer(Proxy, C) ->
     start_bouncer(
         [