Apply spec compliant error formatting

[twist900]

Adds error formatting phase that groups related validation errors under a single group_error code (e.g. BAD_USER_INPUT), following GraphQL spec.

Example Output:

{
  "errors": [
    {
      "message": "Invalid input",
      "extensions": {
        "code": "BAD_USER_INPUT",
        "details": {
          "fields": [
            {
              "message": "min_not_met",
              "path": ["description"],
              "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"
            }
          ]
        }
      }
    }
  ]
}

[twist900]

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.

[mpmiszczyk]

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

[twist900]

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

[andreyuhai]

following GraphQL spec.

Do you mind sharing the link to the spec @twist900? I can't find it mentioned here: https://spec.graphql.org/October2021/

[twist900]

Do you mind sharing the link to the spec @twist900? I can't find it mentioned here: https://spec.graphql.org/October2021/

@andreyuhai I was mostly thinking about including extensions in the errors response, please see here:

GraphQL services may provide an additional entry to errors with key extensions. This entry, if set, must have a map as its value. This entry is reserved for implementers to add additional information to errors however they see fit, and there are no additional restrictions on its contents.

{
  "errors": [
    {
      "message": "Name for character with ID 1002 could not be fetched.",
      "locations": [{ "line": 6, "column": 7 }],
      "path": ["hero", "heroFriends", 1, "name"],
      "extensions": {
        "code": "CAN_NOT_FETCH_BY_ID",
        "timestamp": "Fri Feb 9 14:33:09 UTC 2018"
      }
    }
  ]
}

[mpmiszczyk]

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

[twist900]

package is public :)

[mpmiszczyk]

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

[mpmiszczyk]

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

[twist900]

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

[mpmiszczyk]

Have you thought about adding the changelog? 🙏