From b99569424b91982ee89df6036c8a42507fa31c52 Mon Sep 17 00:00:00 2001 From: James Harton Date: Fri, 15 Mar 2024 12:49:00 +1300 Subject: [PATCH] improvement: Use `Splode` for managing errors. --- .vscode/settings.json | 4 +++- lib/reactor/builder/compose.ex | 3 ++- lib/reactor/error.ex | 13 +++++++++++++ .../{compose_error.ex => internal/compose.ex} | 16 +++++++--------- .../errors/{plan_error.ex => internal/plan.ex} | 16 +++++++++------- .../retries_exceeded.ex} | 10 +++------- lib/reactor/errors/invalid.ex | 7 +++++++ lib/reactor/errors/invalid/transform_error.ex | 10 ++++++++++ lib/reactor/errors/reactor.ex | 7 +++++++ lib/reactor/errors/transform_error.ex | 14 -------------- lib/reactor/errors/unknown.ex | 7 +++++++ lib/reactor/errors/unknown/unknown.ex | 11 +++++++++++ lib/reactor/executor/async.ex | 5 +++-- lib/reactor/executor/sync.ex | 5 +++-- lib/reactor/planner.ex | 3 ++- lib/reactor/step/compose.ex | 3 ++- lib/reactor/step/transform_all.ex | 3 ++- mix.exs | 1 + mix.lock | 1 + test/reactor/builder/compose_test.exs | 13 ++++++++++++- test/reactor/executor/async_test.exs | 5 +++-- test/reactor/executor/sync_test.exs | 7 +++++-- test/reactor/planner_test.exs | 3 ++- 23 files changed, 115 insertions(+), 52 deletions(-) create mode 100644 lib/reactor/error.ex rename lib/reactor/errors/{compose_error.ex => internal/compose.ex} (77%) rename lib/reactor/errors/{plan_error.ex => internal/plan.ex} (78%) rename lib/reactor/errors/{retries_exceeded_error.ex => internal/retries_exceeded.ex} (62%) create mode 100644 lib/reactor/errors/invalid.ex create mode 100644 lib/reactor/errors/invalid/transform_error.ex create mode 100644 lib/reactor/errors/reactor.ex delete mode 100644 lib/reactor/errors/transform_error.ex create mode 100644 lib/reactor/errors/unknown.ex create mode 100644 lib/reactor/errors/unknown/unknown.ex diff --git a/.vscode/settings.json b/.vscode/settings.json index c37723d..ed13369 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -3,6 +3,8 @@ "backoff", "casted", "mappish", - "Planable" + "Planable", + "splode", + "Splode" ] } diff --git a/lib/reactor/builder/compose.ex b/lib/reactor/builder/compose.ex index b17cf62..daf1412 100644 --- a/lib/reactor/builder/compose.ex +++ b/lib/reactor/builder/compose.ex @@ -11,7 +11,8 @@ defmodule Reactor.Builder.Compose do import Reactor, only: :macros import Reactor.Argument, only: :macros import Reactor.Utils - alias Reactor.{Argument, Builder, Error.ComposeError, Step} + alias Reactor.Error.Internal.Compose, as: ComposeError + alias Reactor.{Argument, Builder, Step} @doc """ Compose another Reactor inside this one. diff --git a/lib/reactor/error.ex b/lib/reactor/error.ex new file mode 100644 index 0000000..e6a57a7 --- /dev/null +++ b/lib/reactor/error.ex @@ -0,0 +1,13 @@ +defmodule Reactor.Error do + @moduledoc """ + Uses `splode` to manage various classes of error. + """ + + use Splode, + error_classes: [ + invalid: Reactor.Error.Invalid, + reactor: Reactor.Error.Internal, + unknown: Reactor.Error.Unknown + ], + unknown_error: Reactor.Error.Unknown.Unknown +end diff --git a/lib/reactor/errors/compose_error.ex b/lib/reactor/errors/internal/compose.ex similarity index 77% rename from lib/reactor/errors/compose_error.ex rename to lib/reactor/errors/internal/compose.ex index 5c40046..dbc994d 100644 --- a/lib/reactor/errors/compose_error.ex +++ b/lib/reactor/errors/internal/compose.ex @@ -1,17 +1,15 @@ -defmodule Reactor.Error.ComposeError do +defmodule Reactor.Error.Internal.Compose do @moduledoc """ An error used when attempting to compose to Reactors together. """ - defexception [:outer_reactor, :inner_reactor, :message, :arguments] - import Reactor.Utils - @doc false - @impl true - def exception(attrs), do: struct(__MODULE__, attrs) + use Splode.Error, + fields: [:outer_reactor, :inner_reactor, :message, :arguments], + class: :reactor + + import Reactor.Utils - @doc false - @impl true - def message(error) do + def splode_message(error) do [ """ # Unable to compose Reactors diff --git a/lib/reactor/errors/plan_error.ex b/lib/reactor/errors/internal/plan.ex similarity index 78% rename from lib/reactor/errors/plan_error.ex rename to lib/reactor/errors/internal/plan.ex index 930457a..3073420 100644 --- a/lib/reactor/errors/plan_error.ex +++ b/lib/reactor/errors/internal/plan.ex @@ -1,17 +1,19 @@ -defmodule Reactor.Error.PlanError do +defmodule Reactor.Error.Internal.Plan do @moduledoc """ An error thrown during the planning of a Reactor. """ - defexception [:reactor, :graph, :step, :message] - import Reactor.Utils - @doc false - @impl true - def exception(attrs), do: struct(__MODULE__, attrs) + # defexception [:reactor, :graph, :step, :message] + + use Splode.Error, + fields: [:reactor, :graph, :step, :message], + class: :reactor + + import Reactor.Utils @doc false @impl true - def message(error) do + def splode_message(error) do [ """ # Unable to plan Reactor diff --git a/lib/reactor/errors/retries_exceeded_error.ex b/lib/reactor/errors/internal/retries_exceeded.ex similarity index 62% rename from lib/reactor/errors/retries_exceeded_error.ex rename to lib/reactor/errors/internal/retries_exceeded.ex index 8fadcf2..6d7f6f4 100644 --- a/lib/reactor/errors/retries_exceeded_error.ex +++ b/lib/reactor/errors/internal/retries_exceeded.ex @@ -1,17 +1,13 @@ -defmodule Reactor.Error.RetriesExceededError do +defmodule Reactor.Error.Internal.RetriesExceeded do @moduledoc """ An error used when a step runs out of retry events and no other error is thrown. """ - defexception [:step, :retry_count] + use Splode.Error, fields: [:step, :retry_count], class: :reactor @doc false @impl true - def exception(attrs), do: struct(__MODULE__, attrs) - - @doc false - @impl true - def message(error) do + def splode_message(error) do """ # Maximum number of retries exceeded executing step. diff --git a/lib/reactor/errors/invalid.ex b/lib/reactor/errors/invalid.ex new file mode 100644 index 0000000..286b0b1 --- /dev/null +++ b/lib/reactor/errors/invalid.ex @@ -0,0 +1,7 @@ +defmodule Reactor.Error.Invalid do + use Splode.Error, fields: [:errors], class: :unknown + + def splode_message(%{errors: errors}) do + Splode.ErrorClass.error_messages(errors) + end +end diff --git a/lib/reactor/errors/invalid/transform_error.ex b/lib/reactor/errors/invalid/transform_error.ex new file mode 100644 index 0000000..ba939be --- /dev/null +++ b/lib/reactor/errors/invalid/transform_error.ex @@ -0,0 +1,10 @@ +defmodule Reactor.Error.Invalid.Transform do + @moduledoc """ + An error which occurs when building and running transforms. + """ + use Splode.Error, fields: [:input, :output, :message], class: :invalid + + @doc false + @impl true + def splode_message(error), do: error.message +end diff --git a/lib/reactor/errors/reactor.ex b/lib/reactor/errors/reactor.ex new file mode 100644 index 0000000..fd35aef --- /dev/null +++ b/lib/reactor/errors/reactor.ex @@ -0,0 +1,7 @@ +defmodule Reactor.Error.Internal do + use Splode.Error, fields: [:errors], class: :reactor + + def splode_message(%{errors: errors}) do + Splode.ErrorClass.error_messages(errors) + end +end diff --git a/lib/reactor/errors/transform_error.ex b/lib/reactor/errors/transform_error.ex deleted file mode 100644 index 83833a1..0000000 --- a/lib/reactor/errors/transform_error.ex +++ /dev/null @@ -1,14 +0,0 @@ -defmodule Reactor.Error.TransformError do - @moduledoc """ - An error which occurs when building and running transforms. - """ - defexception input: nil, output: nil, message: nil - - @doc false - @impl true - def exception(attrs), do: struct(__MODULE__, attrs) - - @doc false - @impl true - def message(error), do: error.message -end diff --git a/lib/reactor/errors/unknown.ex b/lib/reactor/errors/unknown.ex new file mode 100644 index 0000000..e864076 --- /dev/null +++ b/lib/reactor/errors/unknown.ex @@ -0,0 +1,7 @@ +defmodule Reactor.Error.Unknown do + use Splode.Error, fields: [:errors], class: :unknown + + def splode_message(%{errors: errors}) do + Splode.ErrorClass.error_messages(errors) + end +end diff --git a/lib/reactor/errors/unknown/unknown.ex b/lib/reactor/errors/unknown/unknown.ex new file mode 100644 index 0000000..7901967 --- /dev/null +++ b/lib/reactor/errors/unknown/unknown.ex @@ -0,0 +1,11 @@ +defmodule Reactor.Error.Unknown.Unknown do + @moduledoc """ + An error used to wrap unknown errors. + """ + + use Splode.Error, fields: [:error], class: :unknown + + def splode_message(%{error: error}) do + Exception.message(error) + end +end diff --git a/lib/reactor/executor/async.ex b/lib/reactor/executor/async.ex index 08dd763..5976eb7 100644 --- a/lib/reactor/executor/async.ex +++ b/lib/reactor/executor/async.ex @@ -3,8 +3,9 @@ defmodule Reactor.Executor.Async do Handle the asynchronous execution of a batch of steps, along with any mutations to the reactor or execution state. """ + alias Reactor.Error.Internal.RetriesExceeded, as: RetriesExceededError alias Reactor.Executor.ConcurrencyTracker - alias Reactor.{Error, Executor, Step} + alias Reactor.{Executor, Step} require Logger @doc """ @@ -276,7 +277,7 @@ defmodule Reactor.Executor.Async do reason {step, :retry} -> - Error.RetriesExceededError.exception( + RetriesExceededError.exception( step: step, retry_count: Map.get(state.retries, step.ref) ) diff --git a/lib/reactor/executor/sync.ex b/lib/reactor/executor/sync.ex index 77a1f73..8bc2b92 100644 --- a/lib/reactor/executor/sync.ex +++ b/lib/reactor/executor/sync.ex @@ -4,7 +4,8 @@ defmodule Reactor.Executor.Sync do the reactor or execution state. """ - alias Reactor.{Error, Executor, Step} + alias Reactor.Error.Internal.RetriesExceeded, as: RetriesExceededError + alias Reactor.{Executor, Step} @doc """ Try and run a step synchronously. @@ -22,7 +23,7 @@ defmodule Reactor.Executor.Sync do reactor = drop_from_plan(reactor, step) error = - Error.RetriesExceededError.exception( + RetriesExceededError.exception( step: step, retry_count: Map.get(state.retries, step.ref) ) diff --git a/lib/reactor/planner.ex b/lib/reactor/planner.ex index 827bf2f..a2d3ca2 100644 --- a/lib/reactor/planner.ex +++ b/lib/reactor/planner.ex @@ -6,7 +6,8 @@ defmodule Reactor.Planner do between them representing their dependencies (arguments). """ - alias Reactor.{Error.PlanError, Step} + alias Reactor.Error.Internal.Plan, as: PlanError + alias Reactor.Step import Reactor, only: :macros import Reactor.Argument, only: :macros import Reactor.Utils diff --git a/lib/reactor/step/compose.ex b/lib/reactor/step/compose.ex index 1487eb5..f132d7c 100644 --- a/lib/reactor/step/compose.ex +++ b/lib/reactor/step/compose.ex @@ -12,7 +12,8 @@ defmodule Reactor.Step.Compose do """ use Reactor.Step - alias Reactor.{Argument, Builder, Error.ComposeError, Info, Step} + alias Reactor.Error.Internal.Compose, as: ComposeError + alias Reactor.{Argument, Builder, Info, Step} import Reactor, only: :macros import Reactor.Argument, only: :macros import Reactor.Utils diff --git a/lib/reactor/step/transform_all.ex b/lib/reactor/step/transform_all.ex index e2e1e96..533e3ac 100644 --- a/lib/reactor/step/transform_all.ex +++ b/lib/reactor/step/transform_all.ex @@ -7,7 +7,8 @@ defmodule Reactor.Step.TransformAll do """ use Reactor.Step - alias Reactor.{Error.TransformError, Step.Transform} + alias Reactor.Error.Invalid.Transform, as: TransformError + alias Reactor.Step.Transform @doc false @impl true diff --git a/mix.exs b/mix.exs index fef4190..966fbbd 100644 --- a/mix.exs +++ b/mix.exs @@ -90,6 +90,7 @@ defmodule Reactor.MixProject do defp deps do [ {:spark, "~> 2.0"}, + {:splode, "~> 0.1.0"}, {:libgraph, "~> 0.16"}, {:telemetry, "~> 1.2"}, diff --git a/mix.lock b/mix.lock index 4d1c287..13d4b44 100644 --- a/mix.lock +++ b/mix.lock @@ -23,6 +23,7 @@ "sobelow": {:hex, :sobelow, "0.13.0", "218afe9075904793f5c64b8837cc356e493d88fddde126a463839351870b8d1e", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "cd6e9026b85fc35d7529da14f95e85a078d9dd1907a9097b3ba6ac7ebbe34a0d"}, "sourceror": {:hex, :sourceror, "1.0.1", "ec2c41726d181adce888ac94b3f33b359a811b46e019c084509e02c70042e424", [:mix], [], "hexpm", "28225464ffd68bda1843c974f3ff7ccef35e29be09a65dfe8e3df3f7e3600c57"}, "spark": {:hex, :spark, "2.0.1", "6701ca7908923767f60d1a9df7274053e2f7c8a0a594452006d038b05193e450", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.0", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "cc3c871dc40184edf2253d1fffcaa931856dac28c6990c4a202bc61f529e266a"}, + "splode": {:hex, :splode, "0.1.0", "235d4c434fe6b011cac3d79c71d78c93f5e5a61ff9fd48ad89a0fa549a28ea65", [:mix], [], "hexpm", "82297087099885345ed656c5b360e261cb2e02c822f6a731d470faf2f50585a5"}, "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"}, "yamerl": {:hex, :yamerl, "0.10.0", "4ff81fee2f1f6a46f1700c0d880b24d193ddb74bd14ef42cb0bcf46e81ef2f8e", [:rebar3], [], "hexpm", "346adb2963f1051dc837a2364e4acf6eb7d80097c0f53cbdc3046ec8ec4b4e6e"}, "yaml_elixir": {:hex, :yaml_elixir, "2.9.0", "9a256da867b37b8d2c1ffd5d9de373a4fda77a32a45b452f1708508ba7bbcb53", [:mix], [{:yamerl, "~> 0.10", [hex: :yamerl, repo: "hexpm", optional: false]}], "hexpm", "0cb0e7d4c56f5e99a6253ed1a670ed0e39c13fc45a6da054033928607ac08dfc"}, diff --git a/test/reactor/builder/compose_test.exs b/test/reactor/builder/compose_test.exs index 29bc36f..109968c 100644 --- a/test/reactor/builder/compose_test.exs +++ b/test/reactor/builder/compose_test.exs @@ -1,6 +1,17 @@ defmodule Reactor.Builder.ComposeTest do use ExUnit.Case, async: true - alias Reactor.{Argument, Builder, Builder.Compose, Error.ComposeError, Planner, Step, Template} + + alias Reactor.Error.Internal.Compose, as: ComposeError + + alias Reactor.{ + Argument, + Builder, + Builder.Compose, + Planner, + Step, + Template + } + require Reactor.Argument describe "compose/4" do diff --git a/test/reactor/executor/async_test.exs b/test/reactor/executor/async_test.exs index 0b2c802..367610f 100644 --- a/test/reactor/executor/async_test.exs +++ b/test/reactor/executor/async_test.exs @@ -1,5 +1,6 @@ defmodule Reactor.Executor.AsyncTest do - alias Reactor.{Error, Executor} + alias Reactor.Error.Internal.RetriesExceeded, as: RetriesExceededError + alias Reactor.Executor import Reactor.Executor.Async use ExUnit.Case, async: true @@ -224,7 +225,7 @@ defmodule Reactor.Executor.AsyncTest do task = Task.Supervisor.async_nolink(supervisor, fn -> :retry end) state = %{state | current_tasks: %{task => undoable}, retries: %{undoable.ref => 100}} - assert {:undo, _reactor, %{errors: [%Error.RetriesExceededError{}]}} = + assert {:undo, _reactor, %{errors: [%RetriesExceededError{}]}} = handle_completed_steps(reactor, state) end end diff --git a/test/reactor/executor/sync_test.exs b/test/reactor/executor/sync_test.exs index c0a26b2..7dfef72 100644 --- a/test/reactor/executor/sync_test.exs +++ b/test/reactor/executor/sync_test.exs @@ -1,5 +1,6 @@ defmodule Reactor.Executor.SyncTest do - alias Reactor.{Error, Executor} + alias Reactor.Error.Internal.RetriesExceeded, as: RetriesExceededError + alias Reactor.Executor import Reactor.Executor.Sync use ExUnit.Case, async: true use Mimic @@ -77,7 +78,9 @@ defmodule Reactor.Executor.SyncTest do |> stub(:run, fn _, _, _ -> :retry end) state = %{state | retries: Map.put(state.retries, step.ref, 100)} - assert {:undo, _, %{errors: [%Error.RetriesExceededError{}]}} = run(reactor, state, step) + + assert {:undo, _, %{errors: [%RetriesExceededError{}]}} = + run(reactor, state, step) end test "when the step is successful it tells the reactor to recurse", %{ diff --git a/test/reactor/planner_test.exs b/test/reactor/planner_test.exs index 78b017a..ed93c98 100644 --- a/test/reactor/planner_test.exs +++ b/test/reactor/planner_test.exs @@ -1,7 +1,8 @@ defmodule Reactor.PlannerTest do @moduledoc false use ExUnit.Case, async: true - alias Reactor.{Builder, Error.PlanError, Info, Planner} + alias Reactor.Error.Internal.Plan, as: PlanError + alias Reactor.{Builder, Info, Planner} describe "plan/1" do test "when the argument is not a reactor, it returns an error" do