Skip to content

Commit 304c237

Browse files
authored
fix: remove unused async options from Reactor.undo/3 (#292)
`Reactor.undo/3` accepted async-related options (`async?`, `max_concurrency`, `timeout`, `max_iterations`) but never used them. The undo operation is intentionally sequential - this is correct saga behaviour as the forward dependency graph cannot be reliably inverted for undo operations. This is a breaking change: passing removed options will now result in a Spark.Options validation error. Also fixes a flaky timing test in executor_test.exs by removing an unnecessarily tight upper bound on elapsed time. Closes #291
1 parent 77c2e39 commit 304c237

File tree

3 files changed

+23
-16
lines changed

3 files changed

+23
-16
lines changed

lib/reactor.ex

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -126,15 +126,7 @@ defmodule Reactor do
126126
| fully_reversible_option
127127
)
128128

129-
@type undo_options ::
130-
Enumerable.t(
131-
max_concurrency_option
132-
| timeout_option
133-
| max_iterations_option
134-
| halt_timeout_option
135-
| async_option
136-
| concurrency_key_option
137-
)
129+
@type undo_options :: Enumerable.t(concurrency_key_option)
138130

139131
@type state :: :pending | :executing | :halted | :failed | :successful
140132
@type inputs :: %{optional(atom) => any}
@@ -253,11 +245,29 @@ defmodule Reactor do
253245
end
254246
end
255247

256-
@undo_options Keyword.drop(@run_schema, [:fully_reversible?])
248+
@undo_options [
249+
concurrency_key: [
250+
type: :reference,
251+
required: false,
252+
hide: true
253+
]
254+
]
257255

258256
@doc """
259257
Attempt to undo a previously successful Reactor.
260258
259+
Undo operations are always executed sequentially in reverse completion order.
260+
This is intentional: while the forward execution graph captures data
261+
dependencies between steps, it cannot capture the full set of constraints
262+
that may apply during rollback. A step's undo logic may have side effects
263+
that affect other steps in ways not expressed in the original dependency
264+
graph, or external systems may have ordering constraints during rollback
265+
that differ from forward execution.
266+
267+
Since undo operations are expected to be infrequent (only triggered on
268+
explicit reversal requests), sequential execution is an acceptable trade-off
269+
that ensures predictable rollback behaviour.
270+
261271
## Arguments
262272
263273
* `reactor` - The previously successful Reactor struct.

lib/reactor/step/compose.ex

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,8 @@ defmodule Reactor.Step.Compose do
3939

4040
@doc false
4141
@impl true
42-
def undo(%{reactor: reactor}, _args, context, options) do
43-
Reactor.undo(reactor, context,
44-
concurrency_key: context.concurrency_key,
45-
async?: options[:allow_async?]
46-
)
42+
def undo(%{reactor: reactor}, _args, context, _options) do
43+
Reactor.undo(reactor, context, concurrency_key: context.concurrency_key)
4744
end
4845

4946
def schedule_undoable_reactor_run(context) do

test/reactor/executor_test.exs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ defmodule Reactor.ExecutorTest do
357357
assert {:ok, _} = Reactor.run(SleepyReactor, %{}, %{}, async?: false)
358358
end)
359359

360-
assert elapsed >= 500 and elapsed <= 600
360+
assert elapsed >= 500
361361
end
362362

363363
test "it can be run asynchronously" do

0 commit comments

Comments
 (0)