GraphQL - Better error messages formatting

big_tests/tests/graphql_SUITE.erl

@@ -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,
@@ -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),
@@ -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(
@@ -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),

big_tests/tests/mongooseimctl_SUITE.erl

@@ -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",

src/mongoose_graphql.erl

@@ -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.
@@ -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() ->

src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl

@@ -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};
@@ -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.
@@ -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)),
@@ -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) ->
@@ -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) ->
@@ -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};
@@ -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}.

src/mongoose_graphql/mongoose_graphql_errors.erl

@@ -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.

src/mongoose_graphql/mongoose_graphql_permissions.erl

@@ -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
@@ -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) ->