From f5fc7ce62595fffda54faf1e0a38a39b92edabff Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Tue, 1 Sep 2026 14:22:17 +0300 Subject: [PATCH 1/6] XYZ-451: Adds for arbitrary `reason_code` in general failure --- .env | 4 ++-- .tool-versions | 2 ++ elvis.config | 8 ++++++-- rebar.config | 9 ++++----- rebar.lock | 2 +- src/payproc_errors.erl | 35 ++++++++++++++++++++++++++--------- test/payproc_errors_SUITE.erl | 22 ++++++++++++++++++++++ 7 files changed, 63 insertions(+), 19 deletions(-) create mode 100644 .tool-versions diff --git a/.env b/.env index d471f87..b2116f2 100644 --- a/.env +++ b/.env @@ -1,3 +1,3 @@ -OTP_VERSION=24.3.4 -REBAR_VERSION=3.18 +OTP_VERSION=28.5.0 +REBAR_VERSION=3.26 THRIFT_VERSION=0.14.2.3 diff --git a/.tool-versions b/.tool-versions new file mode 100644 index 0000000..9455829 --- /dev/null +++ b/.tool-versions @@ -0,0 +1,2 @@ +rebar 3.26.0 +erlang 28.5 diff --git a/elvis.config b/elvis.config index aa53e40..1fd9b38 100644 --- a/elvis.config +++ b/elvis.config @@ -10,7 +10,9 @@ {elvis_text_style, line_length, #{limit => 120}}, {elvis_style, nesting_level, #{level => 3}}, {elvis_style, function_naming_convention, #{regex => "^([a-z][a-z0-9]*_?)*$"}}, - {elvis_style, no_if_expression, disable} + {elvis_style, no_if_expression, disable}, + {elvis_style, export_used_types, disable}, + {elvis_style, no_catch_expressions, disable} ] }, #{ @@ -32,7 +34,9 @@ {elvis_text_style, no_tabs}, {elvis_text_style, no_trailing_whitespace}, %% Temporarily disabled till regex pattern is available - {elvis_project, no_deps_master_rebar, disable} + {elvis_project, no_deps_master_rebar, disable}, + %% TODO Remove it after locking damsel w/ tag + {elvis_project, no_branch_deps, disable} ] }, #{ diff --git a/rebar.config b/rebar.config index 39cbe26..5872635 100644 --- a/rebar.config +++ b/rebar.config @@ -26,7 +26,7 @@ % Common project dependencies. {deps, [ - {damsel, {git, "https://github.com/valitydev/damsel.git", {branch, "master"}}} + {damsel, {git, "https://github.com/valitydev/damsel.git", {branch, "XYZ-451/ft/limit-overflow-failure-details"}}} ]}. %% XRef checks @@ -45,7 +45,6 @@ % mandatory unmatched_returns, error_handling, - race_conditions, unknown ]}, {plt_apps, all_deps} @@ -62,9 +61,9 @@ ]}. {project_plugins, [ - {rebar3_lint, "1.0.1"}, - {erlfmt, "1.0.0"}, - {covertool, "2.0.4"} + {rebar3_lint, "3.2.6"}, + {erlfmt, "1.6.2"}, + {covertool, "2.0.7"} ]}. %% Linter config. diff --git a/rebar.lock b/rebar.lock index 5d15125..8ba1863 100644 --- a/rebar.lock +++ b/rebar.lock @@ -1,4 +1,4 @@ [{<<"damsel">>, {git,"https://github.com/valitydev/damsel.git", - {ref,"dac2cb599499cc0701e60856f4092c9ab283eedf"}}, + {ref,"61e7a831b19b47284a1bd2fdd9cd12bfefb5e306"}}, 0}]. diff --git a/src/payproc_errors.erl b/src/payproc_errors.erl index 43a19e6..0c78d94 100644 --- a/src/payproc_errors.erl +++ b/src/payproc_errors.erl @@ -77,14 +77,22 @@ sub_error_to_static(_, undefined) -> sub_error_to_static(Type, #domain_SubFailure{code = Code, sub = SDE}) -> to_static(Code, Type, SDE). --spec to_static(dynamic_code(), type(), dynamic_sub_error()) -> {static_code(), static_sub_error()}. +-spec to_static(dynamic_code(), type(), dynamic_sub_error()) -> + {static_code(), static_sub_error()} | static_sub_error(). to_static(Code, Type, SDE) -> - StaticCode = code_to_static(Code), - case type_by_field(StaticCode, Type) of - SubType when SubType =/= undefined -> - {StaticCode, sub_error_to_static(SubType, SDE)}; - undefined -> - {{unknown_error, Code}, #payproc_error_GeneralFailure{}} + case code_to_static(Code) of + %% NOTE If were unable to verify code is an existing atom (expected to + %% be defined by compiling damsel protocol), then we consider code an + %% arbitrary reason code from general failure struct. + {unknown_error, ArbitraryCode} when Type =:= 'GeneralFailure' -> + #payproc_error_GeneralFailure{reason_code = ArbitraryCode}; + StaticCode -> + case type_by_field(StaticCode, Type) of + SubType when SubType =/= undefined -> + {StaticCode, sub_error_to_static(SubType, SDE)}; + undefined -> + {{unknown_error, Code}, #payproc_error_GeneralFailure{}} + end end. -spec code_to_static(dynamic_code()) -> static_code(). @@ -121,9 +129,18 @@ code_to_dynamic(Code) -> -spec to_dynamic(type(), static_sub_error()) -> {dynamic_code(), type() | undefined, static_sub_error()}. to_dynamic(_, {Code = {unknown_error, _}, #payproc_error_GeneralFailure{}}) -> {code_to_dynamic(Code), undefined, undefined}; -to_dynamic(Type, {Code, #payproc_error_GeneralFailure{}}) -> +to_dynamic(Type, {Code, #payproc_error_GeneralFailure{reason_code = ReasonCode}}) -> 'GeneralFailure' = check_type(type_by_field(Code, Type)), - {code_to_dynamic(Code), undefined, undefined}; + case ReasonCode of + undefined -> + {code_to_dynamic(Code), undefined, undefined}; + _ -> + %% NOTE It's a special case when general failure have an arbitrary sub code. But + %% when we transform subcode we need another special clause to handle it. + {code_to_dynamic(Code), 'GeneralFailure', {reason_code, ReasonCode}} + end; +to_dynamic('GeneralFailure', {reason_code, ReasonCode}) when is_binary(ReasonCode) -> + {ReasonCode, undefined, undefined}; to_dynamic(Type, {Code, SSE}) -> {code_to_dynamic(Code), check_type(type_by_field(Code, Type)), SSE}. diff --git a/test/payproc_errors_SUITE.erl b/test/payproc_errors_SUITE.erl index 0363305..403b9de 100644 --- a/test/payproc_errors_SUITE.erl +++ b/test/payproc_errors_SUITE.erl @@ -8,6 +8,7 @@ -export([known_error_test/1]). -export([unknown_error_test/1]). -export([unknown_error_atom_test/1]). +-export([general_failure_with_reason_code_test/1]). -export([bad_static_type_test/1]). -export([formatting_test/1]). -export([from_notation_test/1]). @@ -25,6 +26,7 @@ all() -> known_error_test, unknown_error_test, unknown_error_atom_test, + general_failure_with_reason_code_test, bad_static_type_test, formatting_test, from_notation_test, @@ -72,6 +74,26 @@ unknown_error_test(_C) -> DE = payproc_errors:construct('PaymentFailure', SE), ok = payproc_errors:match('PaymentFailure', DE, fun(E) when SE =:= E -> ok end). +-spec general_failure_with_reason_code_test(config()) -> _. +general_failure_with_reason_code_test(_C) -> + DE = #domain_Failure{ + code = <<"no_route_found">>, + sub = #domain_SubFailure{ + code = <<"rejected">>, + sub = #domain_SubFailure{ + code = <<"limit_overflow">>, + sub = #domain_SubFailure{ + code = <<"limit-that-overflowed">> + } + } + } + }, + SE = + {no_route_found, + {rejected, {limit_overflow, #payproc_error_GeneralFailure{reason_code = <<"limit-that-overflowed">>}}}}, + DE = payproc_errors:construct('PaymentFailure', SE), + ok = payproc_errors:match('PaymentFailure', DE, fun(E) when SE =:= E -> ok end). + -spec bad_static_type_test(config()) -> _. bad_static_type_test(_C) -> Bad = {qwe, #payproc_error_GeneralFailure{}}, From d3746704915a5a56005526586feba5232653ec4b Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Tue, 1 Sep 2026 14:24:19 +0300 Subject: [PATCH 2/6] Bumps CI --- .github/workflows/erlang-checks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/erlang-checks.yml b/.github/workflows/erlang-checks.yml index 300ca08..f653c8f 100644 --- a/.github/workflows/erlang-checks.yml +++ b/.github/workflows/erlang-checks.yml @@ -30,7 +30,7 @@ jobs: run: name: Run checks needs: setup - uses: valitydev/erlang-workflows/.github/workflows/erlang-parallel-build.yml@v1.0.10 + uses: valitydev/erlang-workflows/.github/workflows/erlang-parallel-build.yml@v2 with: otp-version: ${{ needs.setup.outputs.otp-version }} rebar-version: ${{ needs.setup.outputs.rebar-version }} From f09631ea4aad57cd9fe3ec52361eaf3dc5d25ba6 Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Tue, 1 Sep 2026 14:44:56 +0300 Subject: [PATCH 3/6] Disables codecov upload --- .github/workflows/erlang-checks.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/erlang-checks.yml b/.github/workflows/erlang-checks.yml index f653c8f..14a351b 100644 --- a/.github/workflows/erlang-checks.yml +++ b/.github/workflows/erlang-checks.yml @@ -36,3 +36,4 @@ jobs: rebar-version: ${{ needs.setup.outputs.rebar-version }} use-thrift: true thrift-version: ${{ needs.setup.outputs.thrift-version }} + upload-coverage: false From 3c3381b59d35bccd97d3edd918863b888cc17a17 Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Fri, 4 Sep 2026 10:14:30 +0300 Subject: [PATCH 4/6] Adds testcase when sub code exists as atom but still translates back --- test/payproc_errors_SUITE.erl | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/payproc_errors_SUITE.erl b/test/payproc_errors_SUITE.erl index 403b9de..37c8605 100644 --- a/test/payproc_errors_SUITE.erl +++ b/test/payproc_errors_SUITE.erl @@ -9,6 +9,7 @@ -export([unknown_error_test/1]). -export([unknown_error_atom_test/1]). -export([general_failure_with_reason_code_test/1]). +-export([general_failure_with_reason_code_existing_atom_test/1]). -export([bad_static_type_test/1]). -export([formatting_test/1]). -export([from_notation_test/1]). @@ -27,6 +28,7 @@ all() -> unknown_error_test, unknown_error_atom_test, general_failure_with_reason_code_test, + general_failure_with_reason_code_existing_atom_test, bad_static_type_test, formatting_test, from_notation_test, @@ -94,6 +96,31 @@ general_failure_with_reason_code_test(_C) -> DE = payproc_errors:construct('PaymentFailure', SE), ok = payproc_errors:match('PaymentFailure', DE, fun(E) when SE =:= E -> ok end). +-spec general_failure_with_reason_code_existing_atom_test(config()) -> _. +general_failure_with_reason_code_existing_atom_test(_C) -> + %% Ensures atom exists + _ = 'Code existing as atom', + DE = #domain_Failure{ + code = <<"no_route_found">>, + sub = #domain_SubFailure{ + code = <<"rejected">>, + sub = #domain_SubFailure{ + code = <<"limit_overflow">>, + sub = #domain_SubFailure{ + code = <<"Code existing as atom">> + } + } + } + }, + SE = + {no_route_found, + {rejected, + {limit_overflow, { + {unknown_error, <<"Code existing as atom">>}, {payproc_error_GeneralFailure, undefined} + }}}}, + DE = payproc_errors:construct('PaymentFailure', SE), + ok = payproc_errors:match('PaymentFailure', DE, fun(E) when SE =:= E -> ok end). + -spec bad_static_type_test(config()) -> _. bad_static_type_test(_C) -> Bad = {qwe, #payproc_error_GeneralFailure{}}, From 9640b25c188b770d83708b091dae82b8595c65fd Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Fri, 4 Sep 2026 10:22:13 +0300 Subject: [PATCH 5/6] Fixes record and linting --- test/payproc_errors_SUITE.erl | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/payproc_errors_SUITE.erl b/test/payproc_errors_SUITE.erl index 37c8605..9fc8b0f 100644 --- a/test/payproc_errors_SUITE.erl +++ b/test/payproc_errors_SUITE.erl @@ -115,9 +115,7 @@ general_failure_with_reason_code_existing_atom_test(_C) -> SE = {no_route_found, {rejected, - {limit_overflow, { - {unknown_error, <<"Code existing as atom">>}, {payproc_error_GeneralFailure, undefined} - }}}}, + {limit_overflow, {{unknown_error, <<"Code existing as atom">>}, #payproc_error_GeneralFailure{}}}}}, DE = payproc_errors:construct('PaymentFailure', SE), ok = payproc_errors:match('PaymentFailure', DE, fun(E) when SE =:= E -> ok end). From 2678fddc8631a89a94ec7e19f12ff3be691f1a37 Mon Sep 17 00:00:00 2001 From: Rustem Shaydullin Date: Fri, 4 Sep 2026 20:51:50 +0500 Subject: [PATCH 6/6] Desired fix --- src/payproc_errors.erl | 31 ++++++++++++++++++------------- test/payproc_errors_SUITE.erl | 15 +++++++++++++-- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/payproc_errors.erl b/src/payproc_errors.erl index 0c78d94..2985948 100644 --- a/src/payproc_errors.erl +++ b/src/payproc_errors.erl @@ -80,19 +80,24 @@ sub_error_to_static(Type, #domain_SubFailure{code = Code, sub = SDE}) -> -spec to_static(dynamic_code(), type(), dynamic_sub_error()) -> {static_code(), static_sub_error()} | static_sub_error(). to_static(Code, Type, SDE) -> - case code_to_static(Code) of - %% NOTE If were unable to verify code is an existing atom (expected to - %% be defined by compiling damsel protocol), then we consider code an - %% arbitrary reason code from general failure struct. - {unknown_error, ArbitraryCode} when Type =:= 'GeneralFailure' -> - #payproc_error_GeneralFailure{reason_code = ArbitraryCode}; - StaticCode -> - case type_by_field(StaticCode, Type) of - SubType when SubType =/= undefined -> - {StaticCode, sub_error_to_static(SubType, SDE)}; - undefined -> - {{unknown_error, Code}, #payproc_error_GeneralFailure{}} - end + StaticCode = code_to_static(Code), + case type_by_field(StaticCode, Type) of + SubType when SubType =/= undefined -> + {StaticCode, sub_error_to_static(SubType, SDE)}; + %% NOTE `GeneralFailure` is a terminal struct: it declares no + %% struct-typed fields, so `type_by_field/2` above yields `undefined` + %% for _any_ code sitting below it. Such a code is an arbitrary reason + %% code by construction, so we decode it as one unconditionally. + %% + %% Deciding this by `binary_to_existing_atom/2` instead would make the + %% result depend on which modules happen to be loaded in this VM: the + %% very same `domain.Failure` would decode to a `reason_code` on one + %% node and to `{unknown_error, _}` on another, forcing every consumer + %% to match both shapes forever. See `general_failure_*` testcases. + undefined when Type =:= 'GeneralFailure' -> + #payproc_error_GeneralFailure{reason_code = Code}; + undefined -> + {{unknown_error, Code}, #payproc_error_GeneralFailure{}} end. -spec code_to_static(dynamic_code()) -> static_code(). diff --git a/test/payproc_errors_SUITE.erl b/test/payproc_errors_SUITE.erl index 9fc8b0f..ab5818a 100644 --- a/test/payproc_errors_SUITE.erl +++ b/test/payproc_errors_SUITE.erl @@ -10,6 +10,7 @@ -export([unknown_error_atom_test/1]). -export([general_failure_with_reason_code_test/1]). -export([general_failure_with_reason_code_existing_atom_test/1]). +-export([general_failure_reason_code_notation_test/1]). -export([bad_static_type_test/1]). -export([formatting_test/1]). -export([from_notation_test/1]). @@ -29,6 +30,7 @@ all() -> unknown_error_atom_test, general_failure_with_reason_code_test, general_failure_with_reason_code_existing_atom_test, + general_failure_reason_code_notation_test, bad_static_type_test, formatting_test, from_notation_test, @@ -114,11 +116,20 @@ general_failure_with_reason_code_existing_atom_test(_C) -> }, SE = {no_route_found, - {rejected, - {limit_overflow, {{unknown_error, <<"Code existing as atom">>}, #payproc_error_GeneralFailure{}}}}}, + {rejected, {limit_overflow, #payproc_error_GeneralFailure{reason_code = <<"Code existing as atom">>}}}}, DE = payproc_errors:construct('PaymentFailure', SE), ok = payproc_errors:match('PaymentFailure', DE, fun(E) when SE =:= E -> ok end). +-spec general_failure_reason_code_notation_test(config()) -> _. +general_failure_reason_code_notation_test(_C) -> + %% NOTE Notation is what downstream cascade and transient error matching + %% keys off, so an arbitrary reason code has to survive the + %% dynamic -> static -> dynamic round trip that `format/2` performs. + Notation = <<"no_route_found:rejected:limit_overflow:limit-that-overflowed">>, + Notation = erlang:list_to_binary( + payproc_errors:format('PaymentFailure', payproc_errors:from_notation(Notation)) + ). + -spec bad_static_type_test(config()) -> _. bad_static_type_test(_C) -> Bad = {qwe, #payproc_error_GeneralFailure{}},