From 361ed7c79bc513d0556a77e126bcc0fe27486449 Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Wed, 22 Feb 2023 10:28:36 +0300 Subject: [PATCH] =?UTF-8?q?TD-431:=20Return=20timeout=20error=20upon=20neg?= =?UTF-8?q?ative=20timeout=20leftover=20after=20end=E2=80=A6=20(#29)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 | 6 +++-- src/bouncer_opa_client.erl | 34 ++++++++++++++---------- test/bouncer_tests_SUITE.erl | 50 +++++++++++++++++++++++++++++++++++- 3 files changed, 73 insertions(+), 17 deletions(-) diff --git a/rebar.config b/rebar.config index aeec88e..2f823d7 100644 --- a/rebar.config +++ b/rebar.config @@ -127,7 +127,9 @@ {test, [ {cover_enabled, true}, - {deps, []}, - {dialyzer, [{plt_extra_apps, [eunit, common_test, gun]}]} + {deps, [ + {meck, "0.9.2"} + ]}, + {dialyzer, [{plt_extra_apps, [eunit, common_test, gun, meck]}]} ]} ]}. diff --git a/src/bouncer_opa_client.erl b/src/bouncer_opa_client.erl index 8a9e191..bd95373 100644 --- a/src/bouncer_opa_client.erl +++ b/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), - %% 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 + do_request_document(TimeoutLeft, ResolvedEndpoint, Path, Body, Headers, Client) 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 diff --git a/test/bouncer_tests_SUITE.erl b/test/bouncer_tests_SUITE.erl index fbab948..d98b21f 100644 --- a/test/bouncer_tests_SUITE.erl +++ b/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( [