diff --git a/lib/ex_graphql/adapter.ex b/lib/ex_graphql/adapter.ex index 5b739bb0b2..a7693f8cdd 100644 --- a/lib/ex_graphql/adapter.ex +++ b/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 diff --git a/lib/ex_graphql/execution.ex b/lib/ex_graphql/execution.ex index d5c1b86c77..ebbc5596da 100644 --- a/lib/ex_graphql/execution.ex +++ b/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 diff --git a/lib/ex_graphql/execution/resolution/field.ex b/lib/ex_graphql/execution/resolution/field.ex index 799aea7e14..daa835c075 100644 --- a/lib/ex_graphql/execution/resolution/field.ex +++ b/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 diff --git a/lib/ex_graphql/execution/variables.ex b/lib/ex_graphql/execution/variables.ex index e4760cfbb8..7263d1c78f 100644 --- a/lib/ex_graphql/execution/variables.ex +++ b/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 diff --git a/test/lib/ex_graphql/execution/variables_test.exs b/test/lib/ex_graphql/execution/variables_test.exs index eecf9c3228..097497ecdb 100644 --- a/test/lib/ex_graphql/execution/variables_test.exs +++ b/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 diff --git a/test/lib/ex_graphql_test.exs b/test/lib/ex_graphql_test.exs index 4699a52bf9..6c997d9329 100644 --- a/test/lib/ex_graphql_test.exs +++ b/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