Skip to content

Commit

Permalink
Merge pull request #3454 from esl/graphql/fix-error-messages
Browse files Browse the repository at this point in the history
GraphQL - Better error messages formatting
  • Loading branch information
chrzaszcz authored Dec 20, 2021
2 parents c218659 + 6d971fe commit 9887260
Show file tree
Hide file tree
Showing 7 changed files with 330 additions and 64 deletions.
66 changes: 57 additions & 9 deletions big_tests/tests/graphql_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ admin_handler() ->
[auth_admin_can_access_protected_types | common_tests()].

common_tests() ->
[wrong_creds_cannot_access_protected_types,
[malformed_auth_header_error,
document_parse_error,
document_type_check_error,
document_validate_error,
wrong_creds_cannot_access_protected_types,
unauth_cannot_access_protected_types,
unauth_can_access_unprotected_types,
can_execute_query_with_variables,
Expand Down Expand Up @@ -86,7 +90,8 @@ unauth_cannot_access_protected_types(Config) ->
Ep = ?config(schema_endpoint, Config),
Body = #{query => "{ field }"},
{Status, Data} = execute(Ep, Body, undefined),
assert_no_permissions(Status, Data).
?assertMatch(#{<<"errors">> := [#{<<"path">> := [<<"ROOT">>]}]}, Data),
assert_no_permissions(no_permissions, Status, Data).

unauth_can_access_unprotected_types(Config) ->
Ep = ?config(schema_endpoint, Config),
Expand All @@ -98,7 +103,46 @@ wrong_creds_cannot_access_protected_types(Config) ->
Ep = ?config(schema_endpoint, Config),
Body = #{query => "{ field }"},
{Status, Data} = execute(Ep, Body, {<<"user">>, <<"wrong_password">>}),
assert_no_permissions(Status, Data).
assert_no_permissions(wrong_credentials, Status, Data).

malformed_auth_header_error(Config) ->
EpName = ?config(schema_endpoint, Config),
Request =
#{port => get_port(EpName),
role => {graphql, atom_to_binary(EpName)},
method => <<"POST">>,
headers => [{<<"Authorization">>, <<"Basic YWRtaW46c2VjcmV">>}],
return_maps => true,
path => "/graphql"},
% The encoded credentials value is malformed and cannot be decoded.
{Status, Data} = rest_helper:make_request(Request),
assert_no_permissions(request_error, Status, Data).

document_parse_error(Config) ->
Ep = ?config(schema_endpoint, Config),
Body = #{<<"query">> => <<"{ field ">>},
{Status, Data} = execute(Ep, Body, undefined),
?assertEqual({<<"400">>,<<"Bad Request">>}, Status),
assert_code(parser_error, Data),

BodyScanner = #{<<"query">> => <<"mutation { id(value: \"asdfsad) } ">>},
{StatusScanner, DataScanner} = execute(Ep, BodyScanner, undefined),
?assertEqual({<<"400">>,<<"Bad Request">>}, StatusScanner),
assert_code(scanner_error, DataScanner).

document_type_check_error(Config) ->
Ep = ?config(schema_endpoint, Config),
Body = #{<<"query">> => <<"mutation { id(value: 12) }">>},
{Status, Data} = execute(Ep, Body, undefined),
?assertEqual({<<"400">>,<<"Bad Request">>}, Status),
assert_code(input_coercion, Data).

document_validate_error(Config) ->
Ep = ?config(schema_endpoint, Config),
Body = #{<<"query">> => <<"query Q1 { field } query Q1 { field }">>, <<"operationName">> => <<"Q1">>},
{Status, Data} = execute(Ep, Body, undefined),
?assertEqual({<<"400">>,<<"Bad Request">>}, Status),
assert_code(not_unique, Data).

auth_user_can_access_protected_types(Config) ->
escalus:fresh_story(
Expand Down Expand Up @@ -148,27 +192,31 @@ invalid_json_body_error(Config) ->
Body = <<"">>,
{Status, Data} = execute(Ep, Body, undefined),
?assertEqual({<<"400">>,<<"Bad Request">>}, Status),
?assertMatch(#{<<"errors">> := [#{<<"message">> := <<"invalid_json_body">>}]}, Data).
assert_code(invalid_json_body, Data).

no_query_supplied_error(Config) ->
Ep = ?config(schema_endpoint, Config),
Body = #{},
{Status, Data} = execute(Ep, Body, undefined),
?assertEqual({<<"400">>,<<"Bad Request">>}, Status),
?assertMatch(#{<<"errors">> := [#{<<"message">> := <<"no_query_supplied">>}]}, Data).
assert_code(no_query_supplied, Data).

variables_invalid_json_error(Config) ->
Ep = ?config(schema_endpoint, Config),
Body = #{<<"query">> => <<"{ field }">>, <<"variables">> => <<"{1: 2}">>},
{Status, Data} = execute(Ep, Body, undefined),
?assertEqual({<<"400">>,<<"Bad Request">>}, Status),
?assertMatch(#{<<"errors">> := [#{<<"message">> := <<"variables_invalid_json">>}]}, Data).
assert_code(variables_invalid_json, Data).

%% Helpers

assert_no_permissions(Status, Data) ->
?assertEqual({<<"400">>,<<"Bad Request">>}, Status),
?assertMatch(#{<<"errors">> := [#{<<"message">> := <<"no_permissions">>}]}, Data).
assert_code(Code, Data) ->
BinCode = atom_to_binary(Code),
?assertMatch(#{<<"errors">> := [#{<<"extensions">> := #{<<"code">> := BinCode}}]}, Data).

assert_no_permissions(ExpectedCode, Status, Data) ->
?assertEqual({<<"401">>,<<"Unauthorized">>}, Status),
assert_code(ExpectedCode, Data).

assert_access_granted(Status, Data) ->
?assertEqual({<<"200">>,<<"OK">>}, Status),
Expand Down
2 changes: 1 addition & 1 deletion big_tests/tests/mongooseimctl_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -1133,7 +1133,7 @@ can_handle_execution_error(Config) ->
Res = mongooseimctl("graphql", [Query], Config),
?assertMatch({_, 0}, Res),
Data = element(1, Res),
?assertNotEqual(nomatch, string:find(Data, "{error,{parser_error")).
?assertNotEqual(nomatch, string:find(Data, "parser_error")).

graphql_wrong_arguments_number(Config) ->
ExpectedFragment = "This command requires",
Expand Down
12 changes: 9 additions & 3 deletions src/mongoose_graphql.erl
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,13 @@ execute(Ep, #{document := Doc,
{ok, graphql:execute(Ep, Ctx2, Ast2)}
catch
throw:{error, Err} ->
{error, Err}
{error, Err};
Class:Reason:Stacktrace ->
Err = #{what => graphql_internal_crash,
class => Class, reason => Reason,
stacktrace => Stacktrace},
?LOG_ERROR(Err),
{error, internal_crash}
end.

%% @doc Execute selected operation on a given endpoint with authorization.
Expand Down Expand Up @@ -104,8 +110,8 @@ graphql_parse(Doc) ->
case graphql:parse(Doc) of
{ok, _} = Ok ->
Ok;
{error, _} = Err ->
throw(Err)
{error, Err} ->
graphql_err:abort([], parse, Err)
end.

admin_mapping_rules() ->
Expand Down
59 changes: 33 additions & 26 deletions src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,19 @@ charsets_provided(Req, State) ->
{[<<"utf-8">>], Req, State}.

is_authorized(Req, State) ->
Auth = cowboy_req:parse_header(<<"authorization">>, Req),
State2 = check_auth(Auth, State),
{true, Req, State2}.
try cowboy_req:parse_header(<<"authorization">>, Req) of
Auth ->
case check_auth(Auth, State) of
{ok, State2} ->
{true, Req, State2};
error ->
Msg = make_error(authorize, wrong_credentials),
reply_error(Msg, Req, State)
end
catch
exit:Err ->
reply_error(make_error(authorize, Err), Req, State)
end.

resource_exists(#{method := <<"GET">>} = Req, State) ->
{true, Req, State};
Expand All @@ -75,7 +85,7 @@ to_html(Req, #{index_location := {priv_file, App, FileLocation}} = State) ->
json_request(Req, State) ->
case gather(Req) of
{error, Reason} ->
reply_error(400, Reason, Req, State);
reply_error(Reason, Req, State);
{ok, Req2, Decoded} ->
run_request(Decoded, Req2, State)
end.
Expand All @@ -92,22 +102,24 @@ check_auth(Auth, #{schema_endpoint := <<"user">>} = State) ->

auth_user({basic, User, Password}, State) ->
case mongoose_api_common:check_password(jid:from_binary(User), Password) of
{true, _} -> State#{authorized => true, schema_ctx => #{username => User}};
_ -> State#{authorized => false}
{true, _} -> {ok, State#{authorized => true, schema_ctx => #{username => User}}};
_ -> error
end;
auth_user(_, State) ->
State#{authorized => false}.
{ok, State#{authorized => false}}.

auth_admin({basic, Username, Password}, #{username := Username, password := Password} = State) ->
State#{authorized => true};
{ok, State#{authorized => true}};
auth_admin({basic, _, _}, _) ->
error;
auth_admin(_, #{username := _, password := _} = State) ->
State#{authorized => false};
{ok, State#{authorized => false}};
auth_admin(_, State) ->
% auth credentials not provided in config
State#{authorized => true}.
{ok, State#{authorized => true}}.

run_request(#{document := undefined}, Req, State) ->
reply_error(400, no_query_supplied, Req, State);
reply_error(make_error(decode, no_query_supplied), Req, State);
run_request(#{} = ReqCtx, Req, #{schema_endpoint := EpName,
authorized := AuthStatus} = State) ->
Ep = mongoose_graphql:get_endpoint(binary_to_existing_atom(EpName)),
Expand All @@ -120,7 +132,7 @@ run_request(#{} = ReqCtx, Req, #{schema_endpoint := EpName,
Reply = cowboy_req:reply(200, Req2),
{stop, Reply, State};
{error, Reason} ->
reply_error(400, Reason, Req, State)
reply_error(Reason, Req, State)
end.

gather(Req) ->
Expand All @@ -131,7 +143,7 @@ gather(Req) ->
gather(Req2, JSON, Bindings)
catch
_:_ ->
{error, invalid_json_body}
{error, make_error(decode, invalid_json_body)}
end.

gather(Req, Body, Params) ->
Expand All @@ -156,10 +168,10 @@ variables([#{<<"variables">> := Vars} | _]) ->
try jiffy:decode(Vars, [return_maps]) of
null -> {ok, #{}};
JSON when is_map(JSON) -> {ok, JSON};
_ -> {error, variables_invalid_json}
_ -> {error, make_error(decode, variables_invalid_json)}
catch
_:_ ->
{error, variables_invalid_json}
{error, make_error(decode, variables_invalid_json)}
end;
is_map(Vars) ->
{ok, Vars};
Expand All @@ -178,17 +190,12 @@ operation_name([_ | Next]) ->
operation_name([]) ->
undefined.

reply_error(Code, Msg, Req, State) ->
Errors =
case Msg of
{error, E} ->
graphql_err:format_errors(#{}, [E]);
_ ->
Formatted = iolist_to_binary(io_lib:format("~p", [Msg])),
[#{type => error, message => Formatted}]
end,

Body = jiffy:encode(#{errors => Errors}),
make_error(Phase, Term) ->
#{error_term => Term, phase => Phase}.

reply_error(Msg, Req, State) ->
{Code, Error} = mongoose_graphql_errors:format_error(Msg),
Body = jiffy:encode(#{errors => [Error]}),
Req2 = cowboy_req:set_resp_body(Body, Req),
Reply = cowboy_req:reply(Code, Req2),
{stop, Reply, State}.
87 changes: 82 additions & 5 deletions src/mongoose_graphql/mongoose_graphql_errors.erl
Original file line number Diff line number Diff line change
@@ -1,18 +1,95 @@
%% @doc Implements callbacks that format custom errors returned from resolvers or crashes.
%% @doc Implements callbacks that format custom errors returned from resolvers or crashes.
%% In addition, it can format each type of error that occurred in any graphql
%% or mongoose_graphql phase.
%% @end
-module(mongoose_graphql_errors).

-export([err/2, crash/2]).
-export([format_error/1, err/2, crash/2]).

-ignore_xref([err/2, crash/2]).
-ignore_xref([format_error/1, err/2, crash/2]).

% callback invoked when resolver returns error tuple
-type err_msg() :: #{message := binary(), extensions => map(), path => list()}.

%% callback invoked when resolver returns error tuple
-spec err(map(), term()) -> err_msg().
err(_Ctx, domain_not_found) ->
#{message => <<"Given domain does not exist">>, extensions => #{code => resolver_error}};
err(_Ctx, ErrorTerm) ->
#{message => iolist_to_binary(io_lib:format("~p", [ErrorTerm])),
extensions => #{code => resolver_error}}.

% callback invoked when resolver crashes
%% callback invoked when resolver crashes
-spec crash(map(), term()) -> err_msg().
crash(_Ctx, #{type := Type}) ->
#{message => <<"Unexpected ", Type/binary, " resolver crash">>,
extensions => #{code => resolver_crash}}.

%% @doc Format error that occurred in any phase including HTTP request decoding.
-spec format_error(term())-> {integer(), err_msg()}.
format_error(#{phase := Phase, error_term := Term} = Err) when Phase =:= authorize;
Phase =:= decode;
Phase =:= parse ->
Msg = #{extensions => #{code => err_code(Phase, Term)},
message => iolist_to_binary(err_msg(Phase, Term))},
{err_http_code(Phase), add_path(Err, Msg)};
format_error(#{error_term := _, phase := Phase} = Err) when Phase =:= execute;
Phase =:= type_check;
Phase =:= validate;
Phase =:= uncategorized ->
Err2 = maps:merge(#{path => []}, Err),
[ErrMsg] = graphql:format_errors(#{}, [Err2]),
{400, ErrMsg};
format_error(internal_crash) ->
Msg = #{message => <<"GraphQL Internal Server Error">>,
extensions => #{code => internal_server_error}},
{500, Msg};
format_error(Err) ->
Msg = #{extensions => #{code => uncategorized},
message => iolist_to_binary(io_lib:format("~p", [Err]))},
{400, Msg}.

%% Internal

err_http_code(authorize) ->
401;
err_http_code(_) ->
400.

err_code(_, Term) ->
simplify(Term).

simplify(A) when is_atom(A) -> A;
simplify(B) when is_binary(B) -> B;
simplify(T) when is_tuple(T) -> element(1, T).

err_msg(parse, Result) ->
parse_err_msg(Result);
err_msg(decode, Result) ->
decode_err_msg(Result);
err_msg(authorize, Result) ->
authorize_err_msg(Result).

authorize_err_msg({request_error, {header, <<"authorization">>}, _}) ->
"Malformed authorization header. Please consult the relevant specification";
authorize_err_msg(wrong_credentials) ->
"The provided credentials are wrong";
authorize_err_msg({no_permissions, Op}) ->
io_lib:format("Cannot execute query ~s without permissions", [Op]).

parse_err_msg({parser_error, {Line, graphql_parser, Msg}}) ->
io_lib:format("Cannot parse line ~B because of ~s", [Line, Msg]);
parse_err_msg({scanner_error, {Line, graphql_scanner, Msg}}) ->
Formatted = lists:flatten(graphql_scanner:format_error(Msg)),
io_lib:format("Cannot scan line ~B because of ~s", [Line, Formatted]).

decode_err_msg(no_query_supplied) ->
"The query was not supplied in the request body";
decode_err_msg(invalid_json_body) ->
"The request JSON body is invalid";
decode_err_msg(variables_invalid_json) ->
"The variables' JSON is invalid".

add_path(#{path := Path}, ErrMsg) ->
ErrMsg#{path => Path};
add_path(_, ErrMsg) ->
ErrMsg.
8 changes: 7 additions & 1 deletion src/mongoose_graphql/mongoose_graphql_permissions.erl
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ check_permissions(OpName, false, #document{definitions = Definitions}) ->
true ->
ok;
false ->
throw({error, no_permissions})
OpName2 = op_name(OpName),
graphql_err:abort([OpName2], authorize, {no_permissions, OpName2})
end;
false ->
ok
Expand All @@ -63,6 +64,11 @@ check_permissions(_, true, _) ->

% Internal

op_name(undefined) ->
<<"ROOT">>;
op_name(Name) ->
Name.

is_req_operation(#op{id = 'ROOT'}, undefined) ->
true;
is_req_operation(#op{id = {name, _, Name}}, Name) ->
Expand Down
Loading

0 comments on commit 9887260

Please sign in to comment.