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

Add the errors key first in the response JSON #131

Merged
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
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