From ef879d00c99a269bd40a5cd82c6f00f7f999f8cd Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Sun, 30 Nov 2008 11:35:14 +0000 Subject: improve error output - consistently display errors as "Error: ..." - display command and args properly formatted when encountering an invalid command --- src/rabbit_control.erl | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/rabbit_control.erl b/src/rabbit_control.erl index 2f7e58e0..4d77bb7d 100644 --- a/src/rabbit_control.erl +++ b/src/rabbit_control.erl @@ -35,23 +35,35 @@ start() -> FullCommand = init:get_plain_arguments(), #params{quiet = Quiet, node = Node, command = Command, args = Args} = - parse_args(FullCommand, #params{quiet = false, node = rabbit_misc:localnode(rabbit)}), + parse_args(FullCommand, #params{quiet = false, + node = rabbit_misc:localnode(rabbit)}), Inform = case Quiet of true -> fun(_Format, _Data) -> ok end; false -> fun io:format/2 end, + %% The reason we don't use a try/catch here is that rpc:call turns + %% thrown errors into normal return values case catch action(Command, Node, Args, Inform) of ok -> Inform("done.~n", []), init:stop(); {'EXIT', {function_clause, [{?MODULE, action, _} | _]}} -> - io:format("Error~nInvalid command ~p~n", [FullCommand]), + error("invalid command '~s'", + [lists:flatten( + rabbit_misc:intersperse( + " ", [atom_to_list(Command) | Args]))]), usage(); + {error, Reason} -> + error("~p", [Reason]), + halt(2); Other -> - io:format("Error~nrabbit_control action ~p failed:~n~p~n", [Command, Other]), + error("~p", [Other]), halt(2) end. +error(Format, Args) -> + io:format("Error: " ++ Format ++"~n", Args). + parse_args(["-n", NodeS | Args], Params) -> Node = case lists:member($@, NodeS) of true -> list_to_atom(NodeS); -- cgit v1.2.1 From 5e7a2dc8b24c7413645b58891d761afdab356023 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Sun, 30 Nov 2008 11:49:10 +0000 Subject: simplify informational display and ensuring consistency also slight tweak to completion message --- src/rabbit_control.erl | 51 +++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/src/rabbit_control.erl b/src/rabbit_control.erl index 4d77bb7d..3540fa90 100644 --- a/src/rabbit_control.erl +++ b/src/rabbit_control.erl @@ -38,14 +38,19 @@ start() -> parse_args(FullCommand, #params{quiet = false, node = rabbit_misc:localnode(rabbit)}), Inform = case Quiet of - true -> fun(_Format, _Data) -> ok end; - false -> fun io:format/2 + true -> fun(_Format, _Args) -> ok end; + false -> fun(Format, Args) -> + io:format(Format ++ " ...~n", Args) + end end, %% The reason we don't use a try/catch here is that rpc:call turns %% thrown errors into normal return values case catch action(Command, Node, Args, Inform) of ok -> - Inform("done.~n", []), + case Quiet of + true -> ok; + false -> io:format("...done.~n") + end, init:stop(); {'EXIT', {function_clause, [{?MODULE, action, _} | _]}} -> error("invalid command '~s'", @@ -121,86 +126,86 @@ output of hostname -s is usually the correct suffix to use after the \"@\" sign. halt(1). action(stop, Node, [], Inform) -> - Inform("Stopping and halting node ~p ...~n", [Node]), + Inform("Stopping and halting node ~p", [Node]), call(Node, {rabbit, stop_and_halt, []}); action(stop_app, Node, [], Inform) -> - Inform("Stopping node ~p ...~n", [Node]), + Inform("Stopping node ~p", [Node]), call(Node, {rabbit, stop, []}); action(start_app, Node, [], Inform) -> - Inform("Starting node ~p ...~n", [Node]), + Inform("Starting node ~p", [Node]), call(Node, {rabbit, start, []}); action(reset, Node, [], Inform) -> - Inform("Resetting node ~p ...~n", [Node]), + Inform("Resetting node ~p", [Node]), call(Node, {rabbit_mnesia, reset, []}); action(force_reset, Node, [], Inform) -> - Inform("Forcefully resetting node ~p ...~n", [Node]), + Inform("Forcefully resetting node ~p", [Node]), call(Node, {rabbit_mnesia, force_reset, []}); action(cluster, Node, ClusterNodeSs, Inform) -> ClusterNodes = lists:map(fun list_to_atom/1, ClusterNodeSs), - Inform("Clustering node ~p with ~p ...~n", + Inform("Clustering node ~p with ~p", [Node, ClusterNodes]), rpc_call(Node, rabbit_mnesia, cluster, [ClusterNodes]); action(status, Node, [], Inform) -> - Inform("Status of node ~p ...~n", [Node]), + Inform("Status of node ~p", [Node]), Res = call(Node, {rabbit, status, []}), io:format("~p~n", [Res]), ok; action(rotate_logs, Node, [], Inform) -> - Inform("Reopening logs for node ~p ...~n", [Node]), + Inform("Reopening logs for node ~p", [Node]), call(Node, {rabbit, rotate_logs, [""]}); action(rotate_logs, Node, Args = [Suffix], Inform) -> - Inform("Rotating logs to files with suffix ~p ...~n", [Suffix]), + Inform("Rotating logs to files with suffix ~p", [Suffix]), call(Node, {rabbit, rotate_logs, Args}); action(add_user, Node, Args = [Username, _Password], Inform) -> - Inform("Creating user ~p ...~n", [Username]), + Inform("Creating user ~p", [Username]), call(Node, {rabbit_access_control, add_user, Args}); action(delete_user, Node, Args = [_Username], Inform) -> - Inform("Deleting user ~p ...~n", Args), + Inform("Deleting user ~p", Args), call(Node, {rabbit_access_control, delete_user, Args}); action(change_password, Node, Args = [Username, _Newpassword], Inform) -> - Inform("Changing password for user ~p ...~n", [Username]), + Inform("Changing password for user ~p", [Username]), call(Node, {rabbit_access_control, change_password, Args}); action(list_users, Node, [], Inform) -> - Inform("Listing users ...~n", []), + Inform("Listing users", []), display_list(call(Node, {rabbit_access_control, list_users, []})); action(add_vhost, Node, Args = [_VHostPath], Inform) -> - Inform("Creating vhost ~p ...~n", Args), + Inform("Creating vhost ~p", Args), call(Node, {rabbit_access_control, add_vhost, Args}); action(delete_vhost, Node, Args = [_VHostPath], Inform) -> - Inform("Deleting vhost ~p ...~n", Args), + Inform("Deleting vhost ~p", Args), call(Node, {rabbit_access_control, delete_vhost, Args}); action(list_vhosts, Node, [], Inform) -> - Inform("Listing vhosts ...~n", []), + Inform("Listing vhosts", []), display_list(call(Node, {rabbit_access_control, list_vhosts, []})); action(map_user_vhost, Node, Args = [_Username, _VHostPath], Inform) -> - Inform("Mapping user ~p to vhost ~p ...~n", Args), + Inform("Mapping user ~p to vhost ~p", Args), call(Node, {rabbit_access_control, map_user_vhost, Args}); action(unmap_user_vhost, Node, Args = [_Username, _VHostPath], Inform) -> - Inform("Unmapping user ~p from vhost ~p ...~n", Args), + Inform("Unmapping user ~p from vhost ~p", Args), call(Node, {rabbit_access_control, unmap_user_vhost, Args}); action(list_user_vhosts, Node, Args = [_Username], Inform) -> - Inform("Listing vhosts for user ~p...~n", Args), + Inform("Listing vhosts for user ~p", Args), display_list(call(Node, {rabbit_access_control, list_user_vhosts, Args})); action(list_vhost_users, Node, Args = [_VHostPath], Inform) -> - Inform("Listing users for vhosts ~p...~n", Args), + Inform("Listing users for vhosts ~p", Args), display_list(call(Node, {rabbit_access_control, list_vhost_users, Args})). display_list(L) when is_list(L) -> -- cgit v1.2.1 From 32d4e29a4631352876b5397118f5887bf9359e3a Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Sun, 30 Nov 2008 12:14:33 +0000 Subject: consistent display of errors in rabbit_multi There is some duplication of code in rabbit_control here, but imho it's too little to justify refactoring --- src/rabbit_multi.erl | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/rabbit_multi.erl b/src/rabbit_multi.erl index c6a7e920..66a560e6 100644 --- a/src/rabbit_multi.erl +++ b/src/rabbit_multi.erl @@ -46,18 +46,22 @@ start() -> io:format("done.~n"), init:stop(); {'EXIT', {function_clause, [{?MODULE, action, _} | _]}} -> - io:format("Invalid command ~p~n", [FullCommand]), + error("invalid command '~s'", + [lists:flatten( + rabbit_misc:intersperse(" ", FullCommand))]), usage(); timeout -> - io:format("timeout starting some nodes.~n"), + error("timeout starting some nodes.", []), halt(1); Other -> - io:format("~nrabbit_multi action ~p failed:~n~p~n", - [Command, Other]), + error("~p", [Other]), halt(2) end end. +error(Format, Args) -> + io:format("Error: " ++ Format ++"~n", Args). + parse_args([Command | Args]) -> {list_to_atom(Command), Args}. -- cgit v1.2.1 From dd05f90819ae2950d5349005b2d4bb1c41219331 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Sun, 30 Nov 2008 12:22:26 +0000 Subject: remove 'error' tags in exceptions since we don't use that tag elsewhere --- src/rabbit_multi.erl | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/rabbit_multi.erl b/src/rabbit_multi.erl index 66a560e6..e2dffa29 100644 --- a/src/rabbit_multi.erl +++ b/src/rabbit_multi.erl @@ -227,8 +227,7 @@ write_pids_file(Pids) -> {ok, Device} -> Device; {error, Reason} -> - throw({error, {cannot_create_pids_file, - FileName, Reason}}) + throw({cannot_create_pids_file, FileName, Reason}) end, try ok = io:write(Handle, Pids), @@ -237,8 +236,7 @@ write_pids_file(Pids) -> case file:close(Handle) of ok -> ok; {error, Reason1} -> - throw({error, {cannot_create_pids_file, - FileName, Reason1}}) + throw({cannot_create_pids_file, FileName, Reason1}) end end, ok. @@ -248,8 +246,7 @@ delete_pids_file() -> case file:delete(FileName) of ok -> ok; {error, enoent} -> ok; - {error, Reason} -> throw({error, {cannot_delete_pids_file, - FileName, Reason}}) + {error, Reason} -> throw({cannot_delete_pids_file, FileName, Reason}) end. read_pids_file() -> @@ -257,8 +254,7 @@ read_pids_file() -> case file:consult(FileName) of {ok, [Pids]} -> Pids; {error, enoent} -> []; - {error, Reason} -> throw({error, {cannot_read_pids_file, - FileName, Reason}}) + {error, Reason} -> throw({cannot_read_pids_file, FileName, Reason}) end. kill_wait(Pid, TimeLeft, Forceful) when TimeLeft < 0 -> -- cgit v1.2.1