improvement: add error context to error creation / normalisation (#440)

This commit is contained in:
Robert Ellen 2022-11-22 09:32:25 +10:00 committed by GitHub
parent d91c3b6d15
commit 59ddb5f5af
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 524 additions and 27 deletions

View file

@ -78,6 +78,108 @@ defmodule Ash.Error do
!!Ash.ErrorKind.impl_for(value)
end
@doc """
Conforms a term into one of the built-in Ash [Error classes](handle-errors.html#error-classes).
The provided term would usually be an Ash Error or a list of Ash Errors.
If the term is:
- a map/struct/Ash Error with a key `:class` having a value `:special`,
- a list with a single map/struct/Ash Error element as above, or
- an `Ash.Error.Invalid` containing such a list in its `:errors` field
then the term is returned unchanged.
Example:
```elixir
iex(1)> Ash.Error.to_error_class("oops", changeset: Ash.Changeset.new(%Post{}), error_context: "some context")
%Ash.Error.Unknown{
changeset: #Ash.Changeset<
errors: [
%Ash.Error.Unknown.UnknownError{
changeset: nil,
class: :unknown,
error: "oops",
error_context: ["some context"],
field: nil,
path: [],
query: nil,
stacktrace: #Stacktrace<>,
vars: []
}
],
...
>,
class: :unknown,
error_context: ["some context"],
errors: [
%Ash.Error.Unknown.UnknownError{
changeset: nil,
class: :unknown,
error: "oops",
error_context: ["some context"],
field: nil,
path: [],
query: nil,
stacktrace: #Stacktrace<>,
vars: []
}
],
stacktrace: #Stacktrace<>,
stacktraces?: true,
vars: []
}
```
Example of nested errors:
```elixir
iex(1)> error1 = Ash.Error.to_ash_error("whoops!", nil, error_context: "some context")
iex(2)> error2 = Ash.Error.to_ash_error("whoops, again!!", nil, error_context: "some other context")
iex(3)> Ash.Error.to_error_class([error1, error2], error_context: "some higher context")
%Ash.Error.Unknown{
changeset: nil,
class: :unknown,
error_context: ["some higher context"],
errors: [
%Ash.Error.Unknown.UnknownError{
changeset: nil,
class: :unknown,
error: "whoops!",
error_context: ["some higher context", "some context"],
field: nil,
path: [],
query: nil,
stacktrace: #Stacktrace<>,
vars: []
},
%Ash.Error.Unknown.UnknownError{
changeset: nil,
class: :unknown,
error: "whoops, again!!",
error_context: ["some higher context", "some other context"],
field: nil,
path: [],
query: nil,
stacktrace: #Stacktrace<>,
vars: []
}
],
path: [],
query: nil,
stacktrace: #Stacktrace<>,
stacktraces?: true,
vars: []
}
```
Options:
- `changeset`: a changeset related to the error
- `query`: a query related to the error
- `error_context`: a sting message providing extra context around the error
"""
def to_error_class(values, opts \\ [])
def to_error_class(%{class: :special} = special, _opts) do
@ -111,51 +213,144 @@ defmodule Ash.Error do
end)
|> Enum.uniq()
choose_error(values, opts[:changeset] || opts[:query])
values
|> accumulate_error_context(opts[:error_context])
|> choose_error(opts[:changeset] || opts[:query])
|> add_error_context(opts[:error_context])
end
end
def to_error_class(value, opts) do
if ash_error?(value) && value.__struct__ in Keyword.values(@error_modules) do
add_changeset_or_query(value, [value], opts[:changeset] || opts[:query])
value
|> add_changeset_or_query([value], opts[:changeset] || opts[:query])
|> Map.put(:error_context, [opts[:error_context] | value.error_context])
else
to_error_class([value], opts)
end
end
def to_ash_error(list, stacktrace \\ nil)
@doc """
Converts a term into an Ash Error.
def to_ash_error(list, stacktrace) when is_list(list) do
The term could be a simple string, the second element in an `{:error, error}` tuple, an Ash Error, or a list of any of these.
In most cases the returned error is an Ash.Error.Unknown.UnknownError.
A stacktrace is added to the error, and any existing stacktrace (i.e. when the term is an Ash Error) is preserved.
`to_ash_error` converts string(s) into UnknownError(s):
```elixir
iex(1)> Ash.Error.to_ash_error("whoops!", nil, error_context: "some context")
%Ash.Error.Unknown.UnknownError{
changeset: nil,
class: :unknown,
error: "whoops!",
error_context: ["some context"],
field: nil,
path: [],
query: nil,
stacktrace: #Stacktrace<>,
vars: []
}
iex(2)> Ash.Error.to_ash_error(["whoops!", "whoops, again!!"], nil, error_context: "some context")
[
%Ash.Error.Unknown.UnknownError{
changeset: nil,
class: :unknown,
error: "whoops!",
error_context: ["some context"],
field: nil,
path: [],
query: nil,
stacktrace: #Stacktrace<>,
vars: []
},
%Ash.Error.Unknown.UnknownError{
changeset: nil,
class: :unknown,
error: "whoops, again!!",
error_context: ["some context"],
field: nil,
path: [],
query: nil,
stacktrace: #Stacktrace<>,
vars: []
}
]
```
`to_ash_error` can preserve error-like data from a keyword-list and accumulate context if called against an Ash Error:
```elixir
iex(1)> err = Ash.Error.to_ash_error([vars: [:some_var], message: "whoops!"], nil, error_context: " some context")
%Ash.Error.Unknown.UnknownError{
changeset: nil,
class: :unknown,
error: "whoops!",
error_context: ["some context"],
field: nil,
path: [],
query: nil,
stacktrace: #Stacktrace<>,
vars: [:some_var]
}
iex(2)> Ash.Error.to_ash_error(err, nil, error_context: "some higher context")
%Ash.Error.Unknown.UnknownError{
changeset: nil,
class: :unknown,
error: "whoops!",
error_context: ["some higher context", "some context"],
field: nil,
path: [],
query: nil,
stacktrace: #Stacktrace<>,
vars: [:some_var]
}
```
Options:
- `error_context`: a sting message providing extra context around the error
"""
def to_ash_error(list, stacktrace \\ nil, opts \\ [])
def to_ash_error(list, stacktrace, opts) when is_list(list) do
if Keyword.keyword?(list) do
error =
list
|> Keyword.take([:error, :vars])
|> Keyword.put_new(:error, list[:message])
|> UnknownError.exception()
add_stacktrace(error, stacktrace)
list
|> Keyword.take([:error, :vars])
|> Keyword.put_new(:error, list[:message])
|> UnknownError.exception()
|> add_stacktrace(stacktrace)
|> add_error_context(opts[:error_context])
else
Enum.map(list, &to_ash_error(&1, stacktrace))
Enum.map(list, &to_ash_error(&1, stacktrace, opts))
end
end
def to_ash_error(error, stacktrace) when is_binary(error) do
add_stacktrace(UnknownError.exception(error: error), stacktrace)
def to_ash_error(error, stacktrace, opts) when is_binary(error) do
[error: error]
|> UnknownError.exception()
|> add_stacktrace(stacktrace)
|> add_error_context(opts[:error_context])
end
def to_ash_error(other, stacktrace) do
def to_ash_error(other, stacktrace, opts) do
cond do
ash_error?(other) ->
add_stacktrace(other, stacktrace)
other
|> add_stacktrace(stacktrace)
|> add_error_context(opts[:error_context])
is_exception(other) ->
error = UnknownError.exception(error: Exception.format(:error, other))
add_stacktrace(error, stacktrace)
[error: Exception.format(:error, other)]
|> UnknownError.exception()
|> add_stacktrace(stacktrace)
|> add_error_context(opts[:error_context])
true ->
error = UnknownError.exception(error: "unknown error: #{inspect(other)}")
add_stacktrace(error, stacktrace)
[error: "unknown error: #{inspect(other)}"]
|> UnknownError.exception()
|> add_stacktrace(stacktrace)
|> add_error_context(opts[:error_context])
end
end
@ -182,6 +377,35 @@ defmodule Ash.Error do
%{error | stacktrace: stacktrace}
end
defp add_error_context(error, error_context) when is_binary(error_context) do
%{error | error_context: [error_context | error.error_context]}
end
defp add_error_context(error, _) do
error
end
defp accumulate_error_context(%{errors: [_ | _] = errors} = error, error_context)
when is_binary(error_context) do
updated_errors = accumulate_error_context(errors, error_context)
%{error | errors: updated_errors}
end
defp accumulate_error_context(errors, error_context)
when is_list(errors) and is_binary(error_context) do
errors
|> Enum.map(fn err ->
err
|> add_error_context(error_context)
|> accumulate_error_context(error_context)
end)
end
defp accumulate_error_context(error, _) do
error
end
@doc "A utility to flatten a list, but preserve keyword list elements"
def flatten_preserving_keywords(list) do
if Keyword.keyword?(list) do
@ -282,7 +506,8 @@ defmodule Ash.Error do
"* #{error}"
%{stacktrace: %Stacktrace{stacktrace: stacktrace}} = class_error ->
"* #{Exception.message(class_error)}\n" <>
breadcrumb(class_error.error_context) <>
"* #{Exception.message(class_error)}\n" <>
path(class_error) <>
Enum.map_join(stacktrace, "\n", fn stack_item ->
" " <> Exception.format_stacktrace_entry(stack_item)
@ -295,7 +520,8 @@ defmodule Ash.Error do
"* #{class_error}"
class_error ->
"* #{Exception.message(class_error)}"
breadcrumb(class_error.error_context) <>
"* #{Exception.message(class_error)}"
end)
end
end)
@ -348,4 +574,11 @@ defmodule Ash.Error do
defp header(:forbidden), do: "Forbidden"
defp header(:framework), do: "Framework Error"
defp header(:unknown), do: "Unknown Error"
defp breadcrumb(nil), do: ""
defp breadcrumb([]), do: ""
defp breadcrumb(error_context) do
"Context: " <> Enum.join(error_context, " > ") <> "\n"
end
end

View file

@ -13,6 +13,7 @@ defmodule Ash.Error.Exception do
[
:changeset,
:query,
error_context: [],
vars: [],
path: [],
stacktrace: [],

View file

@ -236,7 +236,9 @@ defmodule Ash.Filter do
filter
{:error, error} ->
raise Ash.Error.to_error_class(error)
raise Ash.Error.to_error_class(error,
error_context: parse_error_context(resource, statement, context)
)
end
end
@ -929,7 +931,9 @@ defmodule Ash.Filter do
value
{:error, error} ->
raise Ash.Error.to_ash_error(error)
raise Ash.Error.to_ash_error(error, nil,
error_context: parse_error_context(base.resource, addition, context)
)
end
end
@ -966,6 +970,17 @@ defmodule Ash.Filter do
end
end
defp parse_error_context(%{resource: resource} = _filter, addition, context) do
parse_error_context(resource, addition, context)
end
defp parse_error_context(resource, addition, context) do
context_str = if context, do: ", given context: #{inspect(context)}", else: ""
"parsing addition of filter statement: #{inspect(addition)}, to resource: #{inspect(resource)}" <>
context_str
end
@doc """
Returns true if the second argument is a strict subset (always returns the same or less data) of the first
"""

View file

@ -911,10 +911,22 @@ defmodule Ash.Query do
%{query | aggregates: new_aggregates}
else
%{errors: errors} ->
add_error(query, :aggregates, Ash.Error.to_ash_error(errors))
add_error(
query,
:aggregates,
Ash.Error.to_ash_error(errors, nil,
error_context: "Loading aggregate: #{inspect(field)} for query: #{inspect(query)}"
)
)
{:error, error} ->
add_error(query, :aggregates, Ash.Error.to_ash_error(error))
add_error(
query,
:aggregates,
Ash.Error.to_ash_error(error, nil,
error_context: "Loading aggregate: #{inspect(field)} for query: #{inspect(query)}"
)
)
{:can?, false} ->
add_error(

201
test/error_test.exs Normal file
View file

@ -0,0 +1,201 @@
defmodule Ash.Test.ErrorTest do
@moduledoc false
use ExUnit.Case, async: true
defmodule TestError do
use Ash.Error.Exception
def_ash_error([:some_field])
end
defmodule TestResource do
use Ash.Resource, data_layer: Ash.DataLayer.Ets
actions do
defaults [:create, :read, :update, :destroy]
end
attributes do
uuid_primary_key :id
end
end
describe "to_error_class" do
test "returns exception if it is a map/struct with class: :special" do
assert Ash.Error.to_error_class(%{class: :special}, []) == %{class: :special}
end
test "returns exception if it is a map/struct with class: :special wrapped in a list" do
assert Ash.Error.to_error_class([%{class: :special}], []) == %{class: :special}
end
test "returns exception if it is a map/struct with class: :special wrapped in an Invalid error" do
err = Ash.Error.Invalid.exception(errors: [%{class: :special}])
assert Ash.Error.to_error_class(err, []) == %{class: :special}
end
test "returns chosen error if the value argument is a list of values" do
values = [[a: "a", b: "b"], [c: "c", d: "d"], "foo", "bar"]
result = Ash.Error.to_error_class(values, [])
assert match?(%Ash.Error.Unknown{}, result)
# given the test arrangement, each error in the error class should
# * be an Ash.Error.Unknown.UnknownError
# * have a .class == :unknown
# * have a .error that is a distinct element of the uniq subset of the values provided
assert same_elements?(Enum.map(result.errors, fn err -> err.error end), values)
for err <- result.errors do
assert match?(%Ash.Error.Unknown.UnknownError{}, err)
assert err.class == :unknown
end
end
test "returns chosen error if the value argument is a list of errors" do
err1 = Ash.Error.Unknown.UnknownError.exception(error: :an_error)
err2 = Ash.Error.Invalid.exception(errors: [:more, :errors])
result = Ash.Error.to_error_class([err1, err2], [])
# the parent error will be of the invalid class because it take precedence over unknown
assert match?(%Ash.Error.Invalid{}, result)
# the parent error's errors field gets prepended to the list of other errors
assert same_elements?(result.errors, [:more, :errors, err1])
end
test "has a context field populated when there is a single error" do
test_error = TestError.exception([])
err = Ash.Error.to_error_class(test_error, error_context: "some context")
assert err.error_context == ["some context"]
end
test "has a context field populated when there is a list of errors" do
test_error1 = TestError.exception(some_field: :a)
test_error2 = TestError.exception(some_field: :b)
err = Ash.Error.to_error_class([test_error1, test_error2], error_context: "some context")
assert err.error_context == ["some context"]
end
test "accumulates error_context field in child errors" do
error1 = Ash.Error.to_ash_error("whoops!", nil, error_context: "some context")
error2 = Ash.Error.to_ash_error("whoops, again!!", nil, error_context: "some other context")
error_class =
Ash.Error.to_error_class([error1, error2], error_context: "some higher context")
child_error_1 = Enum.find(error_class.errors, fn err -> err.error == "whoops!" end)
assert child_error_1.error_context == ["some higher context", "some context"]
child_error_2 = Enum.find(error_class.errors, fn err -> err.error == "whoops, again!!" end)
assert child_error_2.error_context == ["some higher context", "some other context"]
end
test "accumulates error_context field in child errors who have no error_context of their own" do
error1 = Ash.Error.to_ash_error("whoops!", nil, error_context: "some context")
error2 = Ash.Error.to_ash_error("whoops, again!!", nil)
error_class =
Ash.Error.to_error_class([error1, error2], error_context: "some higher context")
child_error_1 = Enum.find(error_class.errors, fn err -> err.error == "whoops!" end)
assert child_error_1.error_context == ["some higher context", "some context"]
child_error_2 = Enum.find(error_class.errors, fn err -> err.error == "whoops, again!!" end)
assert child_error_2.error_context == ["some higher context"]
end
test "leaves child error contexts unchanged if no error_context field provided" do
error1 = Ash.Error.to_ash_error("whoops!", nil, error_context: "some context")
error2 = Ash.Error.to_ash_error("whoops, again!!", nil, error_context: "some other context")
error_class = Ash.Error.to_error_class([error1, error2])
child_error_1 = Enum.find(error_class.errors, fn err -> err.error == "whoops!" end)
assert child_error_1.error_context == ["some context"]
child_error_2 = Enum.find(error_class.errors, fn err -> err.error == "whoops, again!!" end)
assert child_error_2.error_context == ["some other context"]
end
test "error message contains error context breadcrumbs" do
error1 = Ash.Error.to_ash_error("whoops!", nil, error_context: "some context")
error2 = Ash.Error.to_ash_error("whoops, again!!", nil, error_context: "some other context")
error_class =
Ash.Error.to_error_class([error1, error2], error_context: "some higher context")
error_message = Ash.Error.Unknown.message(error_class)
assert error_message =~ "Context: some higher context > some context"
assert error_message =~ "Context: some higher context > some other context"
end
test "error message still renders when there's no error context" do
error1 = Ash.Error.to_ash_error("whoops!")
error2 = Ash.Error.to_ash_error("whoops, again!!")
error_class = Ash.Error.to_error_class([error1, error2])
error_message = Ash.Error.Unknown.message(error_class)
assert error_message =~ "Unknown Error\n\n* whoops!"
end
test "has a context field populated in changeset" do
test_error = TestError.exception([])
cs = Ash.Changeset.for_create(TestResource, :create)
err = Ash.Error.to_error_class(test_error, changeset: cs, error_context: "some context")
assert err.error_context == ["some context"]
[cs_error] = err.changeset.errors
assert cs_error.error_context == ["some context"]
end
test "accumulates error_context field in changeset's copy of error hierarchy" do
error1 = Ash.Error.to_ash_error("whoops!", nil, error_context: "some context")
error2 = Ash.Error.to_ash_error("whoops, again!!", nil, error_context: "some other context")
cs = Ash.Changeset.for_create(TestResource, :create)
error_class =
Ash.Error.to_error_class([error1, error2],
changeset: cs,
error_context: "some higher context"
)
cs_child_error_1 =
Enum.find(error_class.changeset.errors, fn err -> err.error == "whoops!" end)
cs_child_error_2 =
Enum.find(error_class.changeset.errors, fn err -> err.error == "whoops, again!!" end)
assert cs_child_error_1.error_context == ["some higher context", "some context"]
assert cs_child_error_2.error_context == ["some higher context", "some other context"]
end
end
describe "to_ash_error" do
test "populates error_context field" do
error = Ash.Error.to_ash_error("whoops!", nil, error_context: "some context")
assert error.error_context == ["some context"]
end
end
defp same_elements?(xs, ys) when is_list(xs) and is_list(ys) do
Enum.sort(xs) == Enum.sort(ys)
end
defp same_elements?(_, _), do: false
end

View file

@ -593,6 +593,41 @@ defmodule Ash.Test.Filter.FilterTest do
assert Filter.strict_subset_of?(filter, candidate)
end
test "raises an error if the underlying parse returns an error" do
filter = Filter.parse!(Post, points: 1)
err =
assert_raise(Ash.Error.Query.NoSuchAttributeOrRelationship, fn ->
Filter.add_to_filter!(filter, bad_field: "bad field")
end)
[error_context] = err.error_context
assert error_context =~ "parsing addition of filter statement: [bad_field: \"bad field\"]"
assert error_context =~ ", to resource: " <> (Post |> Module.split() |> Enum.join("."))
end
end
describe "parse!" do
test "raises an error if the statement is invalid" do
filter = Filter.parse!(Post, points: 1)
err =
assert_raise(Ash.Error.Invalid, fn ->
Filter.parse!(filter, 1)
end)
[error_context] = err.error_context
assert error_context =~ "parsing addition of filter statement: 1"
assert error_context =~ ", to resource: " <> (Post |> Module.split() |> Enum.join("."))
[inner_error] = err.errors
[inner_error_context] = inner_error.error_context
assert inner_error_context =~ "parsing addition of filter statement: 1"
assert inner_error_context =~
", to resource: " <> (Post |> Module.split() |> Enum.join("."))
end
end
describe "parse_input" do