TD-568: Pass rejected routes as reason to route_not_found failure (#57)

* Adds comment on `ff_withdrawal:get_quote/1` clauses
* Wraps term with rejected routes reason in tuple for consistency with
other failures reason formatting
This commit is contained in:
Aleksey Kashapov 2023-04-26 10:37:15 +03:00 committed by GitHub
parent cff6ba3aea
commit cc3c9ed8ca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 41 additions and 13 deletions

View File

@ -32,6 +32,8 @@ handle_function_('GetQuote', {MarshaledParams}, _Opts) ->
{ok, Quote} -> {ok, Quote} ->
Response = ff_withdrawal_codec:marshal(quote, Quote), Response = ff_withdrawal_codec:marshal(quote, Quote),
{ok, Response}; {ok, Response};
%% TODO TD-582: Missing clause for routing error?
%% {error, {route, {route_not_found, RejectedRoutes}}} -> ...
{error, {wallet, notfound}} -> {error, {wallet, notfound}} ->
woody_error:raise(business, #fistful_WalletNotFound{}); woody_error:raise(business, #fistful_WalletNotFound{});
{error, {destination, notfound}} -> {error, {destination, notfound}} ->

View File

@ -316,7 +316,7 @@
-type fail_type() :: -type fail_type() ::
limit_check limit_check
| route_not_found | ff_withdrawal_routing:route_not_found()
| {inconsistent_quote_route, {provider_id, provider_id()} | {terminal_id, terminal_id()}} | {inconsistent_quote_route, {provider_id, provider_id()} | {terminal_id, terminal_id()}}
| session. | session.
@ -785,8 +785,8 @@ process_routing(Withdrawal) ->
{continue, [ {continue, [
{route_changed, Route} {route_changed, Route}
]}; ]};
{error, route_not_found} -> {error, {route_not_found, _Rejected} = Reason} ->
Events = process_transfer_fail(route_not_found, Withdrawal), Events = process_transfer_fail(Reason, Withdrawal),
{continue, Events}; {continue, Events};
{error, {inconsistent_quote_route, _Data} = Reason} -> {error, {inconsistent_quote_route, _Data} = Reason} ->
Events = process_transfer_fail(Reason, Withdrawal), Events = process_transfer_fail(Reason, Withdrawal),
@ -799,7 +799,7 @@ process_rollback_routing(Withdrawal) ->
{undefined, []}. {undefined, []}.
-spec do_process_routing(withdrawal_state()) -> {ok, [route()]} | {error, Reason} when -spec do_process_routing(withdrawal_state()) -> {ok, [route()]} | {error, Reason} when
Reason :: route_not_found | attempt_limit_exceeded | InconsistentQuote, Reason :: ff_withdrawal_routing:route_not_found() | attempt_limit_exceeded | InconsistentQuote,
InconsistentQuote :: {inconsistent_quote_route, {provider_id, provider_id()} | {terminal_id, terminal_id()}}. InconsistentQuote :: {inconsistent_quote_route, {provider_id, provider_id()} | {terminal_id, terminal_id()}}.
do_process_routing(Withdrawal) -> do_process_routing(Withdrawal) ->
do(fun() -> do(fun() ->
@ -1237,7 +1237,8 @@ construct_payment_tool(
%% Quote helpers %% Quote helpers
-spec get_quote(quote_params()) -> {ok, quote()} | {error, create_error() | {route, route_not_found}}. -spec get_quote(quote_params()) ->
{ok, quote()} | {error, create_error() | {route, ff_withdrawal_routing:route_not_found()}}.
get_quote(Params = #{destination_id := DestinationID, body := Body, wallet_id := WalletID}) -> get_quote(Params = #{destination_id := DestinationID, body := Body, wallet_id := WalletID}) ->
do(fun() -> do(fun() ->
Destination = unwrap(destination, get_destination(DestinationID)), Destination = unwrap(destination, get_destination(DestinationID)),
@ -1716,7 +1717,7 @@ process_route_change(Withdrawal, Reason) ->
case do_process_routing(Withdrawal) of case do_process_routing(Withdrawal) of
{ok, Routes} -> {ok, Routes} ->
do_process_route_change(Routes, Withdrawal, Reason); do_process_route_change(Routes, Withdrawal, Reason);
{error, route_not_found} -> {error, {route_not_found, _Rejected}} ->
%% No more routes, return last error %% No more routes, return last error
Events = process_transfer_fail(Reason, Withdrawal), Events = process_transfer_fail(Reason, Withdrawal),
{continue, Events} {continue, Events}
@ -1846,10 +1847,15 @@ build_failure(limit_check, Withdrawal) ->
} }
} }
end; end;
build_failure(route_not_found, _Withdrawal) -> build_failure({route_not_found, []}, _Withdrawal) ->
#{ #{
code => <<"no_route_found">> code => <<"no_route_found">>
}; };
build_failure({route_not_found, RejectedRoutes}, _Withdrawal) ->
#{
code => <<"no_route_found">>,
reason => genlib:format({rejected_routes, RejectedRoutes})
};
build_failure({inconsistent_quote_route, {Type, FoundID}}, Withdrawal) -> build_failure({inconsistent_quote_route, {Type, FoundID}}, Withdrawal) ->
Details = Details =
{inconsistent_quote_route, #{ {inconsistent_quote_route, #{

View File

@ -37,8 +37,11 @@
reject_context := reject_context() reject_context := reject_context()
}. }.
-type route_not_found() :: {route_not_found, [ff_routing_rule:rejected_route()]}.
-export_type([route/0]). -export_type([route/0]).
-export_type([routing_context/0]). -export_type([routing_context/0]).
-export_type([route_not_found/0]).
-type identity() :: ff_identity:identity_state(). -type identity() :: ff_identity:identity_state().
-type withdrawal() :: ff_withdrawal:withdrawal_state(). -type withdrawal() :: ff_withdrawal:withdrawal_state().
@ -66,12 +69,12 @@
%% %%
-spec prepare_routes(party_varset(), identity(), domain_revision()) -> -spec prepare_routes(party_varset(), identity(), domain_revision()) ->
{ok, [route()]} | {error, route_not_found}. {ok, [route()]} | {error, route_not_found()}.
prepare_routes(PartyVarset, Identity, DomainRevision) -> prepare_routes(PartyVarset, Identity, DomainRevision) ->
prepare_routes(PartyVarset, #{identity => Identity, domain_revision => DomainRevision, iteration => 1}). prepare_routes(PartyVarset, #{identity => Identity, domain_revision => DomainRevision, iteration => 1}).
-spec prepare_routes(party_varset(), routing_context()) -> -spec prepare_routes(party_varset(), routing_context()) ->
{ok, [route()]} | {error, route_not_found}. {ok, [route()]} | {error, route_not_found()}.
prepare_routes(PartyVarset, Context) -> prepare_routes(PartyVarset, Context) ->
State = gather_routes(PartyVarset, Context), State = gather_routes(PartyVarset, Context),
log_reject_context(State), log_reject_context(State),
@ -143,11 +146,18 @@ get_terminal(#{terminal_id := TerminalID}) ->
TerminalID. TerminalID.
-spec routes(routing_state()) -> -spec routes(routing_state()) ->
{ok, [route()]} | {error, route_not_found}. {ok, [route()]} | {error, route_not_found()}.
routes(#{routes := Routes = [_ | _]}) -> routes(#{routes := Routes = [_ | _]}) ->
{ok, sort_routes(Routes)}; {ok, sort_routes(Routes)};
routes(_) -> routes(#{
{error, route_not_found}. routes := _Routes,
reject_context := #{
varset := _Varset,
accepted_routes := _Accepted,
rejected_routes := Rejected
}
}) ->
{error, {route_not_found, Rejected}}.
-spec get_routes(routing_state()) -> -spec get_routes(routing_state()) ->
[route()]. [route()].

View File

@ -67,6 +67,16 @@
-define(FINAL_BALANCE(Amount, Currency), ?FINAL_BALANCE({Amount, Currency})). -define(FINAL_BALANCE(Amount, Currency), ?FINAL_BALANCE({Amount, Currency})).
-define(assertRouteNotFound(Result, ReasonSubstring), begin
?assertMatch({failed, #{code := <<"no_route_found">>, reason := _Reason}}, Result),
{failed, #{reason := FailureReason}} = Result,
?assert(
nomatch =/= binary:match(FailureReason, ReasonSubstring),
<<"Failure reason '", FailureReason/binary, "' for 'no_route_found' doesn't match '", ReasonSubstring/binary,
"'">>
)
end).
%% API %% API
-spec all() -> [test_case_name() | {group, group_name()}]. -spec all() -> [test_case_name() | {group, group_name()}].
@ -273,7 +283,7 @@ misconfigured_terminal_fail_test(C) ->
}, },
ok = ff_withdrawal_machine:create(WithdrawalParams, ff_entity_context:new()), ok = ff_withdrawal_machine:create(WithdrawalParams, ff_entity_context:new()),
Result = await_final_withdrawal_status(WithdrawalID), Result = await_final_withdrawal_status(WithdrawalID),
?assertMatch({failed, #{code := <<"no_route_found">>}}, Result). ?assertRouteNotFound(Result, <<"{terms_violation,{not_allowed_currency,">>).
-spec limit_check_fail_test(config()) -> test_return(). -spec limit_check_fail_test(config()) -> test_return().
limit_check_fail_test(C) -> limit_check_fail_test(C) ->