From 39640b67c7bc6c46312879beccc54fa5de4c4d95 Mon Sep 17 00:00:00 2001 From: Mickael Remond Date: Sat, 30 Jul 2016 13:08:30 +0200 Subject: [PATCH] Add support for rich error reporting for API --- src/ejabberd_admin.erl | 7 +++---- src/ejabberd_ctl.erl | 11 +++++++++-- src/mod_http_api.erl | 32 +++++++++++++++++++------------- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/ejabberd_admin.erl b/src/ejabberd_admin.erl index f40b99ea5..2c4d5ed6d 100644 --- a/src/ejabberd_admin.erl +++ b/src/ejabberd_admin.erl @@ -380,13 +380,12 @@ register(User, Host, Password) -> {atomic, ok} -> {ok, io_lib:format("User ~s@~s successfully registered", [User, Host])}; {atomic, exists} -> - String = io_lib:format("User ~s@~s already registered at node ~p", - [User, Host, node()]), - {conflict, String}; + Msg = io_lib:format("User ~s@~s already registered", [User, Host]), + {error, conflict, 10090, Msg}; {error, Reason} -> String = io_lib:format("Can't register user ~s@~s at node ~p: ~p", [User, Host, node(), Reason]), - {cannot_register, String} + {error, cannot_register, 10001, String} end. unregister(User, Host) -> diff --git a/src/ejabberd_ctl.erl b/src/ejabberd_ctl.erl index d52d1c0a9..0652267ed 100644 --- a/src/ejabberd_ctl.erl +++ b/src/ejabberd_ctl.erl @@ -212,7 +212,7 @@ process(["help" | Mode], Version) -> end; process(["--version", Arg | Args], _) -> - Version = + Version = try list_to_integer(Arg) catch _:_ -> @@ -321,7 +321,7 @@ call_command([CmdString | Args], Auth, AccessCommands, Version) -> {ArgsFormat, ResultFormat} -> case (catch format_args(Args, ArgsFormat)) of ArgsFormatted when is_list(ArgsFormatted) -> - Result = ejabberd_commands:execute_command(AccessCommands, + Result = ejabberd_commands:execute_command(AccessCommands, Auth, Command, ArgsFormatted, Version), @@ -374,6 +374,12 @@ format_arg2(Arg, Parse)-> format_result({error, ErrorAtom}, _) -> {io_lib:format("Error: ~p", [ErrorAtom]), make_status(error)}; +%% An error should always be allowed to return extended error to help with API. +%% Extended error is of the form: +%% {error, type :: atom(), code :: int(), Desc :: string()} +format_result({error, ErrorAtom, Code, _Msg}, _) -> + {io_lib:format("Error: ~p", [ErrorAtom]), make_status(Code)}; + format_result(Atom, {_Name, atom}) -> io_lib:format("~p", [Atom]); @@ -433,6 +439,7 @@ format_result(404, {_Name, _}) -> make_status(ok) -> ?STATUS_SUCCESS; make_status(true) -> ?STATUS_SUCCESS; +make_status(Code) when is_integer(Code) -> Code; make_status(_Error) -> ?STATUS_ERROR. get_list_commands(Version) -> diff --git a/src/mod_http_api.erl b/src/mod_http_api.erl index f56a47666..6f6d59cda 100644 --- a/src/mod_http_api.erl +++ b/src/mod_http_api.erl @@ -220,12 +220,8 @@ process([Call], #request{method = 'POST', data = Data, ip = {IP, _} = IPPort} = log(Call, Args, IPPort), case check_permissions(Req, Call) of {allowed, Cmd, Auth} -> - case handle(Cmd, Auth, Args, Version, IP) of - {Code, Result} -> - json_response(Code, jiffy:encode(Result)); - {HTMLCode, JSONErrorCode, Message} -> - json_error(HTMLCode, JSONErrorCode, Message) - end; + Result = handle(Cmd, Auth, Args, Version, IP), + json_format(Result); %% Warning: check_permission direcly formats 401 reply if not authorized ErrorResponse -> ErrorResponse @@ -247,8 +243,8 @@ process([Call], #request{method = 'GET', q = Data, ip = IP} = Req) -> log(Call, Args, IP), case check_permissions(Req, Call) of {allowed, Cmd, Auth} -> - {Code, Result} = handle(Cmd, Auth, Args, Version, IP), - json_response(Code, jiffy:encode(Result)); + Result = handle(Cmd, Auth, Args, Version, IP), + json_format(Result); %% Warning: check_permission direcly formats 401 reply if not authorized ErrorResponse -> ErrorResponse @@ -302,7 +298,7 @@ handle(Call, Auth, Args, Version, IP) when is_atom(Call), is_list(Args) -> [{Key, undefined}|Acc] end, [], ArgsSpec), try - handle2(Call, Auth, match(Args2, Spec), Version, IP) + handle2(Call, Auth, match(Args2, Spec), Version, IP) catch throw:not_found -> {404, <<"not_found">>}; throw:{not_found, Why} when is_atom(Why) -> @@ -448,12 +444,12 @@ format_command_result(Cmd, Auth, Result, Version) -> {200, 0}; {{_, rescode}, _} -> {200, 1}; + {_, {error, ErrorAtom, Code, Msg}} -> + format_error_result(ErrorAtom, Code, Msg); {{_, restuple}, {V, Text}} when V == true; V == ok -> {200, iolist_to_binary(Text)}; - {{_, restuple}, {V, Text}} when V == conflict -> - {409, iolist_to_binary(Text)}; - {{_, restuple}, {_, Text2}} -> - {500, iolist_to_binary(Text2)}; + {{_, restuple}, {ErrorAtom, Msg}} -> + format_error_result(ErrorAtom, 0, Msg); {{_, {list, _}}, _V} -> {_, L} = format_result(Result, ResultFormat), {200, L}; @@ -499,6 +495,11 @@ format_result(Tuple, {Name, {tuple, Def}}) -> format_result(404, {_Name, _}) -> "not_found". +format_error_result(conflict, Code, Msg) -> + {409, Code, iolist_to_binary(Msg)}; +format_error_result(_ErrorAtom, Code, Msg) -> + {500, Code, iolist_to_binary(Msg)}. + unauthorized_response() -> json_error(401, 10, <<"Oauth Token is invalid or expired.">>). @@ -507,6 +508,11 @@ badrequest_response() -> badrequest_response(Body) -> json_response(400, jiffy:encode(Body)). +json_format({Code, Result}) -> + json_response(Code, jiffy:encode(Result)); +json_format({HTMLCode, JSONErrorCode, Message}) -> + json_error(HTMLCode, JSONErrorCode, Message). + json_response(Code, Body) when is_integer(Code) -> {Code, ?HEADER(?CT_JSON), Body}.