Skip to content

Commit

Permalink
Add the errors key first in the response JSON
Browse files Browse the repository at this point in the history
According to the `Response Format`
[section](https://facebook.github.io/graphql/June2018/#sec-Response-Format)
of the GraphQL Spec:

> When errors is present in the response, it may be helpful for it to
appear first when serialized to make it more clear when errors are
present in a response during debugging.

refs andreas#119
  • Loading branch information
anmonteiro committed Dec 9, 2018
1 parent 04dccca commit 02868f1
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 34 deletions.
22 changes: 12 additions & 10 deletions graphql/src/graphql_schema.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1148,12 +1148,16 @@ end
in
(`Assoc (("message", `String msg)::props) : Yojson.Basic.json)

let error_response ?path msg =
`Assoc [
"errors", `List [
error_to_json ?path msg
]
let error_response ?data ?path msg =
let errors = "errors", `List [
error_to_json ?path msg
]
in
let data = match data with
| None -> []
| Some data -> ["data", data]
in
`Assoc (errors :: data)

let rec present : type ctx src. ctx execution_context -> src -> Graphql_parser.field -> (ctx, src) typ -> path -> (Yojson.Basic.json * error list, [> resolve_error]) result Io.t =
fun ctx src query_field typ path ->
Expand Down Expand Up @@ -1234,8 +1238,8 @@ end
| data, errors ->
let errors = List.map (fun (msg, path) -> error_to_json ~path msg) errors in
`Assoc [
"errors", `List errors;
"data", data;
"errors", `List errors
]

let to_response = function
Expand All @@ -1253,11 +1257,9 @@ end
| Error (`Validation_error msg) ->
Error (error_response msg)
| Error (`Argument_error msg) ->
let `Assoc errors = error_response msg in
Error (`Assoc (("data", `Null)::errors))
Error (error_response ~data:`Null msg)
| Error (`Resolve_error (msg, path)) ->
let `Assoc errors = error_response ~path msg in
Error (`Assoc (("data", `Null)::errors))
Error (error_response ~data:`Null ~path msg)

let subscribe : type ctx. ctx execution_context -> ctx subscription_field -> Graphql_parser.field -> (Yojson.Basic.json response Io.Stream.t, [> resolve_error]) result Io.t
=
Expand Down
8 changes: 4 additions & 4 deletions graphql/test/argument_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ let suite : (string * [>`Quick] * (unit -> unit)) list = [
("null for required argument", `Quick, fun () ->
let query = "{ input_obj(x: null) }" in
test_query query (`Assoc [
"data", `Null;
"errors", `List [
`Assoc [
"message", `String "Argument `x` of type `person!` expected on field `input_obj`, found null."
]
]
];
"data", `Null;
])
);
("missing optional argument", `Quick, fun () ->
Expand All @@ -87,12 +87,12 @@ let suite : (string * [>`Quick] * (unit -> unit)) list = [
("missing required argument", `Quick, fun () ->
let query = "{ input_obj }" in
test_query query (`Assoc [
"data", `Null;
"errors", `List [
`Assoc [
"message", `String "Argument `x` of type `person!` expected on field `input_obj`, but not provided."
]
]
];
"data", `Null;
])
);
("input coercion: single value to list", `Quick, fun () ->
Expand Down
32 changes: 16 additions & 16 deletions graphql/test/error_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ let suite = [
]) in
let query = "{ nullable }" in
test_query schema () query (`Assoc [
"data", `Assoc [
"nullable", `Null
];
"errors", `List [
`Assoc [
"message", `String "boom";
"path", `List [`String "nullable"]
]
]
];
"data", `Assoc [
"nullable", `Null
];
])
);
("non-nullable error", `Quick, fun () ->
Expand All @@ -31,13 +31,13 @@ let suite = [
]) in
let query = "{ non_nullable }" in
test_query schema () query (`Assoc [
"data", `Null;
"errors", `List [
`Assoc [
"message", `String "boom";
"path", `List [`String "non_nullable"]
]
]
];
"data", `Null;
])
);
("nested nullable error", `Quick, fun () ->
Expand All @@ -58,15 +58,15 @@ let suite = [
in
let query = "{ nullable { non_nullable } }" in
test_query schema () query (`Assoc [
"data", `Assoc [
"nullable", `Null
];
"errors", `List [
`Assoc [
"message", `String "boom";
"path", `List [`String "nullable"; `String "non_nullable"]
]
]
];
"data", `Assoc [
"nullable", `Null
];
])
);
("error in list", `Quick, fun () ->
Expand All @@ -91,6 +91,12 @@ let suite = [
in
let query = "{ foos { id } }" in
test_query schema () query (`Assoc [
"errors", `List [
`Assoc [
"message", `String "boom";
"path", `List [`String "foos"; `Int 2; `String "id"]
]
];
"data", `Assoc [
"foos", `List [
`Assoc [
Expand All @@ -104,12 +110,6 @@ let suite = [
];
]
];
"errors", `List [
`Assoc [
"message", `String "boom";
"path", `List [`String "foos"; `Int 2; `String "id"]
]
]
])
);
]
4 changes: 2 additions & 2 deletions graphql/test/schema_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,13 @@ let suite = [
("subscription returns an error", `Quick, fun () ->
let query = "subscription { subscribe_to_user(error: true) { id name } }" in
test_query query (`Assoc [
"data", `Null;
"errors", `List [
`Assoc [
"message", `String "stream error";
"path", `List [`String "subscribe_to_user"]
]
]
];
"data", `Null;
])
);
("subscriptions: exn inside the stream", `Quick, fun () ->
Expand Down
4 changes: 2 additions & 2 deletions graphql/test/variable_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ let suite : (string * [>`Quick] * (unit -> unit)) list = [
let variables = ["x", `Null] in
let query = "{ input_obj(x: $x) }" in
test_query variables query (`Assoc [
"data", `Null;
"errors", `List [
`Assoc [
"message", `String "Argument `x` of type `person!` expected on field `input_obj`, found null."
]
]
];
"data", `Null;
])
);
("variable coercion: single value to list", `Quick, fun () ->
Expand Down

0 comments on commit 02868f1

Please sign in to comment.