Support error formatting via configured adapter

lib/ex_graphql/adapter.ex

@@ -1,5 +1,7 @@
 defmodule ExGraphQL.Adapter do
 
+  alias ExGraphQL.Execution
+
   defmacro __using__(_) do
     quote do
       @behaviour unquote(__MODULE__)
@@ -14,23 +16,47 @@ defmodule ExGraphQL.Adapter do
       #       support use of adapter in `Execution.format_error`
       def dump_results(internal_results), do: internal_results
 
-      def to_internal_name(_role, external_name), do: external_name
+      def to_internal_name(external_name, _role), do: external_name
+
+      def to_external_name(internal_name, _role), do: internal_name
 
-      def to_external_name(_role, internal_name), do: internal_name
+      def format_error(%{name: name, role: role, value: value}, locations) when is_function(value) do
+        external_name = name |> to_external_name(role)
+        %{
+          message: value.(external_name) |> to_string,
+          locations: locations
+         }
+      end
+      def format_error(%{value: value, role: role} = error_info, locations) when is_binary(value) do
+        role_name = role |> to_string |> String.capitalize
+        %{error_info | value: &"#{role_name} #{&1}: #{value}"}
+        |> format_error(locations)
+      end
+      def format_error(%{value: value} = error_info, locations) do
+        %{error_info | value: inspect(value)}
+        |> format_error(locations)
+      end
 
       defoverridable [load_variables: 1,
                       load_document: 1,
                       dump_results: 1,
+                      format_error: 2,
                       to_internal_name: 2,
                       to_external_name: 2]
     end
   end
 
+  @typedoc "An arbitrarily deep map of variables with binary key names"
+  @type variable_map_t :: %{binary => variable_value_t}
+
+  @typedoc "A value within a variable map"
+  @type variable_value_t :: binary | integer | float | [variable_value_t] | variable_map_t
+
   @doc """
   Convert the incoming (external) variables to the internal representation
   that matches the schema.
   """
-  @callback load_variables(%{binary => any}) :: %{binary => any}
+  @callback load_variables(map) :: variable_map_t
 
   @doc """
   Convert the incoming (external) parsed document to the canonical (internal)
@@ -50,11 +76,26 @@ defmodule ExGraphQL.Adapter do
   @doc """
   Convert a name from an external name to an internal name
   """
-  @callback to_internal_name(role_t, binary) :: binary
+  @callback to_internal_name(binary, role_t) :: binary
 
   @doc """
   Convert a name from an internal name to an external name
   """
-  @callback to_external_name(role_t, binary) :: binary
+  @callback to_external_name(binary, role_t) :: binary
+
+  @doc """
+  Format an error using `value` for `name` located at the provided line/column
+  locations.
+
+  ## Examples
+
+    iex> format_error(%{name: "foo", value: &"missing value '\#{&1}'" end}, [%{line: 2, column: 4}])
+    %{message: "missing value 'foo'", locations: [%{line: 2, column: 4}]}
+
+    iex> format_error(%{name: "foo", value: "missing value"}, [%{line: 2, column: 4}])
+    %{message: "Field 'foo': missing value", locations: [%{line: 2, column: 4}]}
+
+  """
+  @callback format_error(Execution.error_info_t, [Execution.error_location_t]) :: Execution.error_t
 
 end

lib/ex_graphql/execution.ex

@@ -5,7 +5,16 @@ defmodule ExGraphQL.Execution do
 
   alias __MODULE__
 
-  @type error_t :: %{message: binary, locations: [%{line: integer, column: integer}]}
+  @typedoc "The raw information for an error"
+  @type error_info_t :: %{name: binary, role: ExGraphQL.Adapter.role_t, value: ((binary) -> binary) | any}
+
+  @typedoc "A document location for an error"
+  @type error_location_t :: %{line: integer, column: integer}
+
+  @typedoc "The canonical representation of an error, as returned in the result"
+  @type error_t :: %{message: binary, locations: [error_location_t]}
+
+  @typedoc "The canonical result representation of an execution"
   @type result_t :: %{data: %{binary => any}, errors: [error_t]}
 
   @type t :: %{schema: Type.Schema.t, document: Language.Document.t, variables: map, validate: boolean, selected_operation: ExGraphQL.Type.ObjectType.t, operation_name: atom, errors: [error_t], categorized: boolean, strategy: atom, adapter: atom}
@@ -34,12 +43,12 @@ defmodule ExGraphQL.Execution do
 
   @default_adapter ExGraphQL.Adapters.Passthrough
 
-  # Add the configured adapter, if needed
+  @doc "Add the configured adapter to an execution"
   @spec add_configured_adapter(t) :: t
-  defp add_configured_adapter(%{adapter: nil} = execution) do
+  def add_configured_adapter(%{adapter: nil} = execution) do
     %{execution | adapter: configured_adapter}
   end
-  defp add_configured_adapter(execution) do
+  def add_configured_adapter(execution) do
     execution
   end
 
@@ -48,14 +57,17 @@ defmodule ExGraphQL.Execution do
     Application.get_env(:ex_graphql, :adapter, @default_adapter)
   end
 
-  @spec format_error(binary | any, Language.t) :: error_t
-  def format_error(message, %{loc: %{start_line: line}}) when is_binary(message) do
-    %{message: message |> to_string, locations: [%{line: line, column: 0}]}
+  @default_column_number 0
+
+  @spec format_error(atom, error_info_t, Language.t) :: error_t
+  def format_error(%{adapter: adapter}, error_info, %{loc: %{start_line: line}}) do
+    adapter.format_error(error_info, [%{line: line, column: @default_column_number}])
   end
-  def format_error(non_binary_message, %{start_line: _} = ast_node) do
-    non_binary_message
-    |> inspect
-    |> format_error(ast_node)
+
+  @spec format_error(binary, Language.t) :: error_t
+  @doc "Format an error, without using the adapter (useful when reporting on types and other unadapted names)"
+  def format_error(message, %{loc: %{start_line: line}}) do
+    %{message: message, locations: [%{line: line, column: @default_column_number}]}
   end
 
   @spec resolve_type(t, t, t) :: t | nil
@@ -143,8 +155,8 @@ defmodule ExGraphQL.Execution do
     {:error, "Multiple operations available, but no operation_name provided"}
   end
 
-  def set_variables(%{schema: schema, selected_operation: selected_op, variables: variables} = execution) do
-    case Execution.Variables.build(schema, selected_op.variable_definitions, variables) do
+  def set_variables(%{selected_operation: selected_op, variables: variables} = execution) do
+    case Execution.Variables.build(execution, selected_op.variable_definitions, variables) do
       %{values: values, errors: new_errors} ->
         %{execution | variables: values, errors: new_errors ++ execution.errors}
     end

lib/ex_graphql/execution/resolution/field.ex

@@ -18,7 +18,9 @@ defimpl ExGraphQL.Execution.Resolution, for: ExGraphQL.Language.Field do
           |> process_raw_result(ast_node, field, resolution, execution)
       end
     else
-      {:skip, %{execution | errors: [Execution.format_error("No field '#{ast_node.name}'", ast_node)|errors]}}
+      error_info = %{name: ast_node.name, role: :field, value: "Not present in schema"}
+      error = Execution.format_error(execution, error_info, ast_node)
+      {:skip, %{execution | errors: [error|errors]}}
     end
   end
 
@@ -27,11 +29,21 @@ defimpl ExGraphQL.Execution.Resolution, for: ExGraphQL.Language.Field do
     |> result(ast_node, field, resolution, execution)
   end
   defp process_raw_result({:error, error}, ast_node, _field, _resolution, execution) do
-    new_errors = error |> List.wrap |> Enum.map(&(Execution.format_error(&1, ast_node)))
+    new_errors = error
+    |> List.wrap
+    |> Enum.map(fn (value) ->
+      error_info = %{name: ast_node.name, role: :field, value: value}
+      Execution.format_error(execution, error_info, ast_node)
+    end)
     {:skip, %{execution | errors: new_errors ++ execution.errors }}
   end
   defp process_raw_result(_other, ast_node, _field, _resolution, execution) do
-    error = "Schema did not resolve '#{ast_node.name}' to match {:ok, _} or {:error, _}" |> Execution.format_error(ast_node)
+    error_info = %{
+      name: ast_node.name,
+      role: :field,
+      value: "Did not resolve to match {:ok, _} or {:error, _}"
+    }
+    error = Execution.format_error(execution, error_info, ast_node)
     {:skip, %{execution | errors: [error|execution.errors]}}
   end
 

lib/ex_graphql/execution/variables.ex

@@ -6,15 +6,15 @@ defmodule ExGraphQL.Execution.Variables do
   alias ExGraphQL.Execution.LiteralInput
 
   @spec build(Type.Schema.t, [Language.VariableDefinition.t], %{binary => any}) :: %{binary => any}
-  def build(schema, variable_definitions, provided_variables) do
+  def build(execution, variable_definitions, provided_variables) do
     parsed = variable_definitions
-    |> parse(schema, provided_variables |> Execution.stringify_keys, %{errors: [], values: %{}})
+    |> parse(execution, provided_variables |> Execution.stringify_keys, %{errors: [], values: %{}})
   end
 
-  defp parse([], schema, provided_variables, acc) do
+  defp parse([], execution, provided_variables, acc) do
     acc
   end
-  defp parse([definition|rest], schema, provided_variables, %{errors: errors, values: values} = acc) do
+  defp parse([definition|rest], %{schema: schema} = execution, provided_variables, %{errors: errors, values: values} = acc) do
     variable_name = definition.variable.name
     %{name: type_name} = unwrapped_definition_type = definition.type |> Language.unwrap
     variable_type = Type.Schema.type_from_ast(schema, definition.type)
@@ -31,25 +31,35 @@ defmodule ExGraphQL.Execution.Variables do
           end
         end
         parse(
-          rest, schema, provided_variables,
+          rest, execution, provided_variables,
           %{acc | values: values |> Map.put(variable_name, coerced)}
         )
       else
         err = if is_nil(value) do
-          "Missing required variable '#{variable_name}' (#{type_name})"
+          &"Variable #{&1} (#{type_name}): Not provided"
         else
-          "Invalid value for variable '#{variable_name}' (#{type_name}): #{inspect value}"
+          &"Variable #{&1} (#{type_name}): Invalid value"
         end
-        error = Execution.format_error(err, unwrapped_definition_type)
+        error_info = %{
+          name: variable_name,
+          role: :variable,
+          value: err
+        }
+        error = Execution.format_error(execution, error_info, unwrapped_definition_type)
         parse(
-          rest, schema, provided_variables,
+          rest, execution, provided_variables,
           %{acc | errors: [error|errors]}
         )
       end
     else
-      error = Execution.format_error("Could not find type '#{type_name}' in schema", unwrapped_definition_type)
+      error_info = %{
+        name: variable_name,
+        role: :variable,
+        value: "Type (#{type_name}) not present in schema"
+      }
+      error = Execution.format_error(execution, error_info, unwrapped_definition_type)
       parse(
-        rest, schema, provided_variables,
+        rest, execution, provided_variables,
         %{acc | errors: [error|errors]}
       )
     end

test/lib/ex_graphql/execution/variables_test.exs

@@ -26,12 +26,13 @@ defmodule ExGraphQL.Execution.VariablesTest do
     # Get schema
     schema = StarWars.Schema.schema
     # Prepare execution context
-    {:ok, selected_op} = %Execution{schema: schema, document: document}
+    execution = %Execution{schema: schema, document: document}
     |> Execution.categorize_definitions
-    |> Execution.selected_operation
+    |> Execution.add_configured_adapter
+    {:ok, selected_op} = Execution.selected_operation(execution)
     # Build variable map
     Execution.Variables.build(
-      schema,
+      execution,
       selected_op.variable_definitions,
       provided
     )
@@ -51,7 +52,7 @@ defmodule ExGraphQL.Execution.VariablesTest do
     context "when not provided" do
 
       it "returns an error" do
-        assert %{values: %{}, errors: [%{message: "Missing required variable 'id' (String)"}]} = @id_required |> variables
+        assert %{values: %{}, errors: [%{message: "Variable id (String): Not provided"}]} = @id_required |> variables
       end
 
     end

test/lib/ex_graphql_test.exs

@@ -105,7 +105,7 @@ defmodule ExGraphQLTest do
       }
     }
     """
-    assert {:ok, %{data: %{"thing" => %{"name" => "Foo"}}, errors: [%{message: "No field 'bad'", locations: [%{line: 4}]}]}} = ExGraphQL.run(simple_schema, query, validate: false)
+    assert {:ok, %{data: %{"thing" => %{"name" => "Foo"}}, errors: [%{message: "Field bad: Not present in schema", locations: [%{line: 4, column: 0}]}]}} = ExGraphQL.run(simple_schema, query, validate: false)
   end
 
   it "gives nice errors for bad resolutions" do
@@ -115,7 +115,7 @@ defmodule ExGraphQLTest do
     }
     """
     assert {:ok, %{data: %{},
-                   errors: [%{message: "Schema did not resolve 'bad_resolution' to match {:ok, _} or {:error, _}", locations: _}]}} = ExGraphQL.run(simple_schema, query, validate: false)
+                   errors: [%{message: "Field bad_resolution: Did not resolve to match {:ok, _} or {:error, _}", locations: _}]}} = ExGraphQL.run(simple_schema, query, validate: false)
   end
 
   it "returns the correct results for an alias" do
@@ -152,7 +152,7 @@ defmodule ExGraphQLTest do
       }
     """
     assert {:ok, %{data: %{"thingByContext" => %{"name" => "Bar"}}, errors: []}} = ExGraphQL.run(simple_schema, query, validate: false, context: %{thing: "bar"})
-    assert {:ok, %{data: %{}, errors: [%{message: "No :id context provided"}]}} = ExGraphQL.run(simple_schema, query, validate: false)
+    assert {:ok, %{data: %{}, errors: [%{message: "Field thingByContext: No :id context provided"}]}} = ExGraphQL.run(simple_schema, query, validate: false)
   end
 
   it "can use variables" do
@@ -167,7 +167,6 @@ defmodule ExGraphQLTest do
     assert {:ok, %{data: %{"thing" => %{"name" => "Bar"}}, errors: []}} = result
   end
 
-  @tag :focus
   it "reports missing, required variable values" do
     query = """
       query GimmeThingByVariable($thingId: String!, $other: String!) {
@@ -177,7 +176,7 @@ defmodule ExGraphQLTest do
       }
     """
     result = ExGraphQL.run(simple_schema, query, validate: false, variables: %{thingId: "bar"})
-    assert {:ok, %{data: %{"thing" => %{"name" => "Bar"}}, errors: [%{message: "Missing required variable 'other' (String)"}]}} = result
+    assert {:ok, %{data: %{"thing" => %{"name" => "Bar"}}, errors: [%{message: "Variable other (String): Not provided"}]}} = result
   end
 
 end