From 292d4196991f6783e5f6d844bf3e194685e46f4d Mon Sep 17 00:00:00 2001 From: beamologist Date: Tue, 23 Jun 2026 05:51:33 +0200 Subject: [PATCH 1/3] khepri_machine: propagate process_command errors from readwrite transactions When process_command/3 returns {error, Reason} (e.g. {error, noproc} during Ra initialisation), the catch-all clause 'Ret -> {ok, Ret}' was wrapping it as {ok, {error, Reason}}. Callers that pattern-match on {ok, Value} would then silently receive an error tuple as the transaction result, causing crashes or wrong behaviour downstream. Add an explicit {error, _} = Error -> Error clause before the catch-all so infrastructure errors are propagated directly rather than being mistaken for a successful transaction return value. --- src/khepri_machine.erl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/khepri_machine.erl b/src/khepri_machine.erl index 9b5aec6a..809f51a4 100644 --- a/src/khepri_machine.erl +++ b/src/khepri_machine.erl @@ -591,6 +591,8 @@ readwrite_transaction1(StoreId, StandaloneFunOrPath, Args, Options) -> case process_command(StoreId, Command, Options1) of {exception, _, _, _} = Exception -> handle_tx_exception(Exception); + {error, _} = Error -> + Error; ok = Ret -> CommandType = select_command_type(Options), case CommandType of From bb6d043e1e1672c37e437d6940dc1f4568aaf527 Mon Sep 17 00:00:00 2001 From: beamologist Date: Wed, 24 Jun 2026 07:09:30 +0200 Subject: [PATCH 2/3] test: cover process_command error propagation in readwrite transactions Add a regression test asserting that a read-write transaction against a store that is not running returns {error, noproc} directly, rather than the wrapped {ok, {error, noproc}} that the catch-all clause previously produced. A path-pattern transaction is used so the command reaches process_command/3 without stand-alone function extraction (which would require the khepri application to be started). The test fails without the {error, _} = Error -> Error clause and passes with it. --- test/simple_tx_misc.erl | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/simple_tx_misc.erl b/test/simple_tx_misc.erl index 212785f6..40018d78 100644 --- a/test/simple_tx_misc.erl +++ b/test/simple_tx_misc.erl @@ -696,3 +696,19 @@ tx_using_erl_eval_test_() -> fun local_fun_using_erl_eval/0, rw) end)]}. + +readwrite_transaction_propagates_infra_error_test() -> + %% When the store is not running, `process_command/3' returns an + %% infrastructure error such as `{error, noproc}'. + %% `readwrite_transaction1/3' must propagate that error tuple directly + %% instead of wrapping it as `{ok, {error, noproc}}'; a caller matching + %% on `{ok, Value}' would otherwise mistake the error for a successful + %% transaction result and crash downstream. + %% + %% A path-pattern transaction is used so the command reaches + %% `process_command/3' without going through stand-alone function + %% extraction. `?FUNCTION_NAME' is a store ID that is never started, so + %% the command cannot reach a Ra server. + ?assertEqual( + {error, noproc}, + khepri:transaction(?FUNCTION_NAME, [foo], [], rw)). From 4d30f501519aa371adef9a5a8af7417de5f6e4f8 Mon Sep 17 00:00:00 2001 From: beamologist Date: Fri, 26 Jun 2026 12:33:44 +0200 Subject: [PATCH 3/3] khepri_machine: also propagate process_command errors from read-only transactions The read-write fix left the same defect in both readonly_transaction/4 clauses. process_query/3 can return an infrastructure error such as {error, noproc} (store not running) or {error, timeout}, but the catch-all 'Ret -> {ok, Ret}' wrapped it as {ok, {error, Reason}}. A caller matching on {ok, Value} would then mistake the error for a successful transaction result. Add an explicit {error, _} = Error -> Error clause to both read-only clauses so dispatch errors propagate verbatim, matching the read-write path and khepri:handle_async_ret/2 (which already returns {error, _} directly and raises only {exception, _, _, _}). tx_ret() already permits {error, Reason} via tx_abort(), so the public shape is unchanged. Document tx_ret() and add a read-only regression test mirroring the existing read-write one against a non-running store. --- src/khepri_machine.erl | 14 ++++++++++++++ test/simple_tx_misc.erl | 12 ++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/khepri_machine.erl b/src/khepri_machine.erl index 809f51a4..0ed9eec1 100644 --- a/src/khepri_machine.erl +++ b/src/khepri_machine.erl @@ -242,6 +242,16 @@ -type tx_ret() :: khepri:ok(khepri_tx:tx_fun_result()) | khepri_tx:tx_abort() | no_return(). +%% Return value of a transaction. +%% +%% A successful transaction returns `{ok, Result}'. A failure is reported as a +%% bare `{error, Reason}' tuple, never wrapped in `{ok, _}'. This covers both +%% an aborted transaction (see {@link khepri_tx:abort/1}) and an infrastructure +%% error surfaced while dispatching the transaction (for example `{error, +%% noproc}' or `{error, timeout}' when the store is not running). This matches +%% how {@link khepri:handle_async_ret/2} propagates the same errors for +%% asynchronous transactions. To return an error-shaped value as a successful +%% result, wrap it explicitly, e.g. `{ok, {error, Reason}}'. -type async_ret() :: ok. @@ -541,6 +551,8 @@ readonly_transaction(StoreId, Fun, Args, Options) case process_query(StoreId, Query, Options) of {exception, _, _, _} = Exception -> handle_tx_exception(Exception); + {error, _} = Error -> + Error; Ret -> {ok, Ret} end; @@ -559,6 +571,8 @@ readonly_transaction(StoreId, PathPattern, Args, Options) case process_query(StoreId, Query, Options) of {exception, _, _, _} = Exception -> handle_tx_exception(Exception); + {error, _} = Error -> + Error; Ret -> {ok, Ret} end. diff --git a/test/simple_tx_misc.erl b/test/simple_tx_misc.erl index 40018d78..470e072a 100644 --- a/test/simple_tx_misc.erl +++ b/test/simple_tx_misc.erl @@ -712,3 +712,15 @@ readwrite_transaction_propagates_infra_error_test() -> ?assertEqual( {error, noproc}, khepri:transaction(?FUNCTION_NAME, [foo], [], rw)). + +readonly_transaction_propagates_infra_error_test() -> + %% Same expectation as the read-write case, but for a read-only + %% transaction going through `process_query/3'. The infrastructure error + %% must be propagated verbatim rather than wrapped as + %% `{ok, {error, noproc}}'. + %% + %% `?FUNCTION_NAME' is a store ID that is never started, so the query + %% cannot reach a Ra server. + ?assertEqual( + {error, noproc}, + khepri:transaction(?FUNCTION_NAME, [foo], [], ro)).