gdamjan / erlang-irc-bot

A simple extendable irc bot in Erlang
http://github.com/gdamjan/erlang-irc-bot/wiki
MIT License
80 stars 25 forks source link

Silent module failing is no fun. #15

Closed davidw closed 14 years ago

davidw commented 14 years ago

If there's an error in your module's initialization, it fails silently. This is no fun at all, and quite common because of erlang's weird module behavior (you have to import everything from the 'top level' that you want to use).

davidw commented 14 years ago

Here's a quick patch:

--- a/src/ircbot_server.erl
+++ b/src/ircbot_server.erl
@@ -68,7 +68,7 @@ handle_call(disconnect, _From, {Self, State, Config}) ->

 handle_call({add_plugin, Plugin, Args}, _From, {Self, State, Config}) ->
-    gen_event:add_handler(State#state.plugin_mgr, Plugin, Args),
+    ok = gen_event:add_handler(State#state.plugin_mgr, Plugin, Args),
     {reply, ok, {Self, State, Config}};

 handle_call({delete_plugin, Plugin, Args}, _From, {Self, State, Config}) ->
gdamjan commented 14 years ago

I agree that silent module failing is no fun... I hoped to solve that when I implement full OTP application support (and sasl).

Otherwise, I think "ok = gen_event:add_handler(...)" will kill the gen_server if it fails. Have you tried it?

davidw commented 14 years ago

I'd rather have it fail than have no error message of any kind. I guess it wouldn't be that hard to handle the other cases though... I'll see about coding that up.

davidw commented 14 years ago

How about this:

case gen_event:add_handler(State#state.plugin_mgr, Plugin, Args) of
ok ->
    ok;
{'EXIT', Reason} ->
    error_logger:error_msg("Problem loading handler ~p ~p ~n", [Plugin, Reason]);
Other ->
    error_logger:error_msg("Loading ~p reports ~p ~n", [Plugin, Other])
end,
gdamjan commented 14 years ago

seems good, I'll test it and commit it

gdamjan commented 14 years ago

pushed in 26a8810cb696ffdab348d1b6c758754a0df86067