Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply spec compliant error formatting #10

Merged
merged 1 commit into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 60 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
# Absinthe Helpers

This package provides two key features:
This package provides three key features:

1. **constraints**: enforce validation rules (like `min`, `max`, etc.) on fields and arguments in your schema.
2. **transforms**: apply custom transformations (like `Trim`, `ToInteger`, etc.) to input fields and arguments.
1. **constraints**: enforce validation rules (like `min`, `max`, etc.) on fields and arguments in your schema
2. **transforms**: apply custom transformations (like `Trim`, `ToInteger`, etc.) to input fields and arguments
3. **error formatting**: GraphQL specification compliant error formatting for validation and transform failures

## Installation

Expand All @@ -23,11 +24,11 @@ Then, run:
mix deps.get
```

### Setup: adding constraints and transforms to your Absinthe pipeline
### Setup: adding constraints, transforms, and error formatting to your Absinthe pipeline

To set up both **constraints** and **transforms**, follow these steps:
Follow these steps:

1. Add constraints and transforms to your Absinthe pipeline:
1. Add constraints, transforms, and error formatting to your Absinthe pipeline:

```elixir
forward "/graphql",
Expand All @@ -42,6 +43,7 @@ def absinthe_pipeline(config, opts) do
|> Absinthe.Plug.default_pipeline(opts)
|> AbsintheHelpers.Phases.ApplyConstraints.add_to_pipeline(opts)
|> AbsintheHelpers.Phases.ApplyTransforms.add_to_pipeline(opts)
|> AbsintheHelpers.Phases.ApplyErrorFormatting.add_to_pipeline(opts)
end
```

Expand Down Expand Up @@ -147,3 +149,55 @@ field(:create_booking, :string) do
resolve(&TestResolver.run/3)
end
```

## Error Formatting

The package includes GraphQL specification compliant error formatting. When enabled, validation errors from constraints or transform failures are formatted consistently.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package is internal, isn't it? Can we link to RFC?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

package is public :)


For multiple related errors, they are grouped under a single error with BAD_USER_INPUT code:

```json
{
"errors": [
{
"message": "Invalid input",
"extensions": {
"code": "BAD_USER_INPUT",
"details": {
"fields": [
{
"message": "min_not_met",
"path": ["description"],
Copy link
Member Author

@twist900 twist900 Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the path currently includes only the field name, we'd need to figure out how to have the full path including nesting and indexing into lists here. I've seen something here:

https://hexdocs.pm/absinthe/Absinthe.Resolution.html#path/1

but its seems to be available once the schema is resolved, which happens later in the pipeline. That said, happy to look into it separately again.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. FYI don't use path_string - it doesn't work with list (positinos in list are integers and 😭)
  2. we could try making it middleware rather then pipeline; there you have access to resolution

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI don't use path_string - it doesn't work with list (positinos in list are integers and 😭)

the link is indeed to path 👍

we could try making it middleware rather then pipeline; there you have access to resolution

unfortunately these errors cause early exit in the Absinthe pipeline, i.e. they never reach the Execution phase where the middlewares are called

"details": {
"min": 5
},
"locations": [
{
"line": 6,
"column": 7
}
],
"custom_error_code": "min_not_met"
},
{
"message": "max_exceeded",
"path": ["title"],
"details": {
"max": 10
},
"locations": [
{
"line": 7,
"column": 7
}
],
"custom_error_code": "max_exceeded"
}
]
}
},
"locations": []
}
]
}
```
6 changes: 5 additions & 1 deletion lib/phases/apply_constraints.ex
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,12 @@ defmodule AbsintheHelpers.Phases.ApplyConstraints do
Phase.put_error(node, %Phase.Error{
phase: __MODULE__,
message: reason,
locations: [node.source_location],
extra: %{
details: Map.merge(details, %{field: node.name})
group_code: :BAD_USER_INPUT,
code: reason,
path: [node.name],
details: details
}
})
end
Expand Down
69 changes: 69 additions & 0 deletions lib/phases/apply_error_formatting.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
defmodule AbsintheHelpers.Phases.ApplyErrorFormatting do
@moduledoc false

alias Absinthe.{Blueprint, Phase}
use Absinthe.Phase

def add_to_pipeline(pipeline, opts) do
Absinthe.Pipeline.insert_after(
pipeline,
Phase.Document.Result,
{__MODULE__, opts}
)
end

@spec run(Blueprint.t() | Phase.Error.t(), Keyword.t()) :: {:ok, Blueprint.t()}
def run(blueprint = %Blueprint{}, _options) do
{:ok, %{blueprint | result: format_errors(blueprint.result)}}
end

defp format_errors(%{errors: errors} = result) when is_list(errors) do
%{result | errors: format_error_list(errors)}
end

defp format_errors(result), do: result

defp format_error_list(errors) do
errors
|> Enum.group_by(&get_error_code/1)
|> Enum.flat_map(&format_error_group/1)
end

defp get_error_code(%{group_code: group_code}), do: group_code
defp get_error_code(_), do: nil

defp format_error_group({nil, errors}), do: errors

defp format_error_group({group_code, errors}) do
[
%{
message: "Invalid input",
locations: [],
extensions: %{
code: group_code,
details: %{
fields: Enum.map(errors, &format_field/1)
}
}
}
]
end
Comment on lines +29 to +50

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be idiomatic; you still would like to have one entry per group

Suggested change
|> Enum.flat_map(&format_error_group/1)
end
defp get_error_code(%{group_code: group_code}), do: group_code
defp get_error_code(_), do: nil
defp format_error_group({nil, errors}), do: errors
defp format_error_group({group_code, errors}) do
[
%{
message: "Invalid input",
locations: [],
extensions: %{
code: group_code,
details: %{
fields: Enum.map(errors, &format_field/1)
}
}
}
]
end
|> Enum.map(&format_error_group/1)
end
defp get_error_code(%{group_code: group_code}), do: group_code
defp get_error_code(_), do: nil
defp format_error_group({nil, errors}), do: errors
defp format_error_group({group_code, errors}) do
%{
message: "Invalid input",
locations: [],
extensions: %{
code: group_code,
details: %{
fields: Enum.map(errors, &format_field/1)
}
}
}
end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what is intended by this suggestion, should line 36 also format the errors as a single map? otherwise in one case we return a list in another a map


defp format_field(%{
message: message,
locations: locations,
path: path,
code: code,
details: details
}) do
%{
message: message,
path: path,
custom_error_code: code,
details: details,
locations: locations
}
end

defp format_field(error), do: error
end
6 changes: 5 additions & 1 deletion lib/phases/apply_transforms.ex
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,12 @@ defmodule AbsintheHelpers.Phases.ApplyTransforms do
Absinthe.Phase.put_error(node, %Absinthe.Phase.Error{
phase: __MODULE__,
message: reason,
locations: [node.source_location],
extra: %{
details: Map.merge(details, %{field: node.name})
group_code: :BAD_USER_INPUT,
code: reason,
path: [node.name],
details: details
}
})
end
Expand Down
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ defmodule AbsintheHelpers.MixProject do
def project do
[
app: :absinthe_helpers,
version: "0.1.9",
version: "0.2.8",
elixir: "~> 1.16",
start_permanent: Mix.env() == :prod,
deps: deps(),
Expand Down
81 changes: 60 additions & 21 deletions test/absinthe_helpers/phases/apply_constraints_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -91,26 +91,61 @@ defmodule AbsintheHelpers.Phases.ApplyConstraintsTest do
}
"""

assert {:ok,
%{
errors: [
%{
message: :max_exceeded,
details: %{field: "customer_id", max: 1000}
},
%{message: :min_not_met, details: %{field: "cost", min: 10}},
%{message: :min_not_met, details: %{field: "description", min: 5}},
%{message: :min_not_met, details: %{field: "override_ids", min: 5}},
%{
message: :min_items_not_met,
details: %{field: "location_ids", min_items: 2}
},
%{
message: :max_items_exceeded,
details: %{field: "commission_ids", max_items: 2}
}
]
}} = TestSchema.run_query(query)
assert {
:ok,
%{
errors: [
%{
details: %{max: 1000},
message: :max_exceeded,
code: :max_exceeded,
group_code: :BAD_USER_INPUT,
locations: [%{line: 3, column: 5}],
path: ["customer_id"]
},
%{
details: %{min: 10},
message: :min_not_met,
code: :min_not_met,
group_code: :BAD_USER_INPUT,
locations: [%{line: 5, column: 7}],
path: ["cost"]
},
%{
details: %{min: 5},
message: :min_not_met,
code: :min_not_met,
group_code: :BAD_USER_INPUT,
locations: [%{line: 6, column: 7}],
path: ["description"]
},
%{
details: %{min: 5},
message: :min_not_met,
code: :min_not_met,
group_code: :BAD_USER_INPUT,
locations: [%{line: 7, column: 7}],
path: ["override_ids"]
},
%{
details: %{min_items: 2},
message: :min_items_not_met,
code: :min_items_not_met,
group_code: :BAD_USER_INPUT,
locations: [%{line: 8, column: 7}],
path: ["location_ids"]
},
%{
details: %{max_items: 2},
message: :max_items_exceeded,
code: :max_items_exceeded,
group_code: :BAD_USER_INPUT,
locations: [%{line: 9, column: 7}],
path: ["commission_ids"]
}
]
}
} = TestSchema.run_query(query)
end

test "returns invalid_format on strings that do not match regex pattern" do
Expand All @@ -133,8 +168,12 @@ defmodule AbsintheHelpers.Phases.ApplyConstraintsTest do
%{
errors: [
%{
details: %{regex: "^[a-zA-Z ]+$"},
message: :invalid_format,
details: %{field: "description", regex: "^[a-zA-Z\s]+$"}
code: :invalid_format,
group_code: :BAD_USER_INPUT,
locations: [%{line: 6, column: 7}],
path: ["description"]
}
]
}} = TestSchema.run_query(query)
Expand Down
Loading