Browse Source

Changes suggested by Kostis, Dialyzer -Wunmatched_returns and Tidier

pull/46/merge
Andrew Thompson 13 years ago
parent
commit
efc7b7591b
9 changed files with 73 additions and 51 deletions
  1. +1
    -1
      Makefile
  2. +4
    -2
      src/error_logger_lager_h.erl
  3. +23
    -19
      src/lager.erl
  4. +13
    -7
      src/lager_app.erl
  5. +3
    -2
      src/lager_crash_log.erl
  6. +7
    -5
      src/lager_file_backend.erl
  7. +12
    -6
      src/lager_handler_watcher.erl
  8. +1
    -2
      src/lager_trunc_io.erl
  9. +9
    -7
      src/lager_util.erl

+ 1
- 1
Makefile View File

@ -39,7 +39,7 @@ dialyzer: compile
@echo Use "'make build_plt'" to build PLT prior to using this target. @echo Use "'make build_plt'" to build PLT prior to using this target.
@echo @echo
@sleep 1 @sleep 1
dialyzer -Wno_return --plt $(COMBO_PLT) ebin | \
dialyzer -Wno_return -Wunmatched_returns --plt $(COMBO_PLT) ebin | \
fgrep -v -f ./dialyzer.ignore-warnings fgrep -v -f ./dialyzer.ignore-warnings
cleanplt: cleanplt:

+ 4
- 2
src/error_logger_lager_h.erl View File

@ -35,14 +35,16 @@
-define(LOG(Level, Pid, Msg), -define(LOG(Level, Pid, Msg),
case ?SHOULD_LOG(Level) of case ?SHOULD_LOG(Level) of
true -> true ->
lager:log(Level, Pid, Msg);
_ =lager:log(Level, Pid, Msg),
ok;
_ -> ok _ -> ok
end). end).
-define(LOG(Level, Pid, Fmt, Args), -define(LOG(Level, Pid, Fmt, Args),
case ?SHOULD_LOG(Level) of case ?SHOULD_LOG(Level) of
true -> true ->
lager:log(Level, Pid, Fmt, Args);
_ = lager:log(Level, Pid, Fmt, Args),
ok;
_ -> ok _ -> ok
end). end).

+ 23
- 19
src/lager.erl View File

@ -125,7 +125,7 @@ trace_file(File, Filter, Level) ->
{ok, Trace} -> {ok, Trace} ->
Handlers = gen_event:which_handlers(lager_event), Handlers = gen_event:which_handlers(lager_event),
%% check if this file backend is already installed %% check if this file backend is already installed
case lists:member({lager_file_backend, File}, Handlers) of
Res = case lists:member({lager_file_backend, File}, Handlers) of
false -> false ->
%% install the handler %% install the handler
supervisor:start_child(lager_handler_watcher_sup, supervisor:start_child(lager_handler_watcher_sup,
@ -133,14 +133,20 @@ trace_file(File, Filter, Level) ->
_ -> _ ->
ok ok
end, end,
%% install the trace.
{MinLevel, Traces} = lager_mochiglobal:get(loglevel),
case lists:member(Trace, Traces) of
false ->
case Res of
{ok, _} ->
%% install the trace.
{MinLevel, Traces} = lager_mochiglobal:get(loglevel),
case lists:member(Trace, Traces) of
false ->
lager_mochiglobal:put(loglevel, {MinLevel, [Trace|Traces]}); lager_mochiglobal:put(loglevel, {MinLevel, [Trace|Traces]});
_ -> ok
end,
{ok, Trace};
_ ->
ok
end,
{ok, Trace};
{error, _} = E ->
E
end;
Error -> Error ->
Error Error
end. end.
@ -184,15 +190,14 @@ stop_trace({_Filter, _Level, Target} = Trace) ->
clear_all_traces() -> clear_all_traces() ->
{MinLevel, _Traces} = lager_mochiglobal:get(loglevel), {MinLevel, _Traces} = lager_mochiglobal:get(loglevel),
lager_mochiglobal:put(loglevel, {MinLevel, []}), lager_mochiglobal:put(loglevel, {MinLevel, []}),
[begin
case get_loglevel(Handler) of
none ->
gen_event:delete_handler(lager_event, Handler, []);
_ ->
ok
end
end || Handler <- gen_event:which_handlers(lager_event)],
ok.
lists:foreach(fun(Handler) ->
case get_loglevel(Handler) of
none ->
gen_event:delete_handler(lager_event, Handler, []);
_ ->
ok
end
end, gen_event:which_handlers(lager_event)).
status() -> status() ->
Handlers = gen_event:which_handlers(lager_event), Handlers = gen_event:which_handlers(lager_event),
@ -283,8 +288,7 @@ safe_format(Fmt, Args, Limit) ->
safe_format(Fmt, Args, Limit, []). safe_format(Fmt, Args, Limit, []).
safe_format(Fmt, Args, Limit, Options) -> safe_format(Fmt, Args, Limit, Options) ->
try lager_trunc_io:format(Fmt, Args, Limit, Options) of
Result -> Result
try lager_trunc_io:format(Fmt, Args, Limit, Options)
catch catch
_:_ -> lager_trunc_io:format("FORMAT ERROR: ~p ~p", [Fmt, Args], Limit) _:_ -> lager_trunc_io:format("FORMAT ERROR: ~p ~p", [Fmt, Args], Limit)
end. end.

+ 13
- 7
src/lager_app.erl View File

@ -43,7 +43,8 @@ start(_StartType, _StartArgs) ->
Val Val
end, end,
[supervisor:start_child(lager_handler_watcher_sup, [lager_event, Module, Config]) ||
%% handlers failing to start are handled in the handler_watcher
_ = [supervisor:start_child(lager_handler_watcher_sup, [lager_event, Module, Config]) ||
{Module, Config} <- expand_handlers(Handlers)], {Module, Config} <- expand_handlers(Handlers)],
%% mask the messages we have no use for %% mask the messages we have no use for
@ -55,18 +56,23 @@ start(_StartType, _StartArgs) ->
{ok, false} -> {ok, false} ->
[]; [];
_ -> _ ->
supervisor:start_child(lager_handler_watcher_sup, [error_logger, error_logger_lager_h, []]),
%% Should we allow user to whitelist handlers to not be removed?
[begin error_logger:delete_report_handler(X), X end ||
X <- gen_event:which_handlers(error_logger) -- [error_logger_lager_h]]
case supervisor:start_child(lager_handler_watcher_sup, [error_logger, error_logger_lager_h, []]) of
{ok, _} ->
%% Should we allow user to whitelist handlers to not be removed?
[begin error_logger:delete_report_handler(X), X end ||
X <- gen_event:which_handlers(error_logger) -- [error_logger_lager_h]];
{error, _} ->
[]
end
end, end,
{ok, Pid, SavedHandlers}. {ok, Pid, SavedHandlers}.
stop(Handlers) -> stop(Handlers) ->
[error_logger:add_report_handler(Handler) || Handler <- Handlers],
ok.
lists:foreach(fun(Handler) ->
error_logger:add_report_handler(Handler)
end, Handlers).
expand_handlers([]) -> expand_handlers([]) ->
[]; [];

+ 3
- 2
src/lager_crash_log.erl View File

@ -110,9 +110,10 @@ code_change(_OldVsn, State, _Extra) ->
{ok, State}. {ok, State}.
schedule_rotation(undefined) -> schedule_rotation(undefined) ->
undefined;
ok;
schedule_rotation(Date) -> schedule_rotation(Date) ->
erlang:send_after(lager_util:calculate_next_rotation(Date) * 1000, self(), rotate).
erlang:send_after(lager_util:calculate_next_rotation(Date) * 1000, self(), rotate),
ok.
%% ===== Begin code lifted from riak_err ===== %% ===== Begin code lifted from riak_err =====

+ 7
- 5
src/lager_file_backend.erl View File

@ -108,8 +108,8 @@ handle_info(_Info, State) ->
%% @private %% @private
terminate(_Reason, #state{fd=FD}) -> terminate(_Reason, #state{fd=FD}) ->
%% flush and close any file handles %% flush and close any file handles
file:datasync(FD),
file:close(FD),
_ = file:datasync(FD),
_ = file:close(FD),
ok. ok.
%% @private %% @private
@ -123,7 +123,8 @@ write(#state{name=Name, fd=FD, inode=Inode, flap=Flap, size=RotSize,
lager_util:rotate_logfile(Name, Count), lager_util:rotate_logfile(Name, Count),
write(State, Level, Msg); write(State, Level, Msg);
{ok, {NewFD, NewInode, _}} -> {ok, {NewFD, NewInode, _}} ->
file:write(NewFD, Msg),
%% delayed_write doesn't report errors
_ = file:write(NewFD, Msg),
case Level of case Level of
_ when Level =< ?ERROR -> _ when Level =< ?ERROR ->
%% force a sync on any message at error severity or above %% force a sync on any message at error severity or above
@ -196,9 +197,10 @@ validate_logfile(H) ->
false. false.
schedule_rotation(_, undefined) -> schedule_rotation(_, undefined) ->
undefined;
ok;
schedule_rotation(Name, Date) -> schedule_rotation(Name, Date) ->
erlang:send_after(lager_util:calculate_next_rotation(Date) * 1000, self(), {rotate, Name}).
erlang:send_after(lager_util:calculate_next_rotation(Date) * 1000, self(), {rotate, Name}),
ok.
-ifdef(TEST). -ifdef(TEST).

+ 12
- 6
src/lager_handler_watcher.erl View File

@ -61,9 +61,14 @@ handle_info({gen_event_EXIT, Module, shutdown}, #state{module=Module} = State) -
{stop, normal, State}; {stop, normal, State};
handle_info({gen_event_EXIT, Module, Reason}, #state{module=Module, handle_info({gen_event_EXIT, Module, Reason}, #state{module=Module,
config=Config, event=Event} = State) -> config=Config, event=Event} = State) ->
lager:log(error, self(), "Lager event handler ~p exited with reason ~s",
[Module, error_logger_lager_h:format_reason(Reason)]),
install_handler(Event, Module, Config),
case lager:log(error, self(), "Lager event handler ~p exited with reason ~s",
[Module, error_logger_lager_h:format_reason(Reason)]) of
ok ->
install_handler(Event, Module, Config);
{error, _} ->
%% lager is not working, so installing a handler won't work
ok
end,
{noreply, State}; {noreply, State};
handle_info(reinstall_handler, #state{module=Module, config=Config, event=Event} = State) -> handle_info(reinstall_handler, #state{module=Module, config=Config, event=Event} = State) ->
install_handler(Event, Module, Config), install_handler(Event, Module, Config),
@ -82,13 +87,14 @@ code_change(_OldVsn, State, _Extra) ->
install_handler(Event, Module, Config) -> install_handler(Event, Module, Config) ->
case gen_event:add_sup_handler(Event, Module, Config) of case gen_event:add_sup_handler(Event, Module, Config) of
ok -> ok ->
lager:log(debug, self(), "Lager installed handler ~p into ~p", [Module, Event]),
_ = lager:log(debug, self(), "Lager installed handler ~p into ~p", [Module, Event]),
ok; ok;
Error -> Error ->
%% try to reinstall it later %% try to reinstall it later
lager:log(error, self(), "Lager failed to install handler ~p into"
_ = lager:log(error, self(), "Lager failed to install handler ~p into"
" ~p, retrying later : ~p", [Module, Event, Error]), " ~p, retrying later : ~p", [Module, Event, Error]),
erlang:send_after(5000, self(), reinstall_handler)
erlang:send_after(5000, self(), reinstall_handler),
ok
end. end.
-ifdef(TEST). -ifdef(TEST).

+ 1
- 2
src/lager_trunc_io.erl View File

@ -60,8 +60,7 @@ format(Fmt, Args, Max) ->
format(Fmt, Args, Max, []). format(Fmt, Args, Max, []).
format(Fmt, Args, Max, Options) -> format(Fmt, Args, Max, Options) ->
try lager_format:format(Fmt, Args, Max, Options) of
Result -> Result
try lager_format:format(Fmt, Args, Max, Options)
catch catch
_:_ -> _:_ ->
erlang:error(badarg, [Fmt, Args]) erlang:error(badarg, [Fmt, Args])

+ 9
- 7
src/lager_util.erl View File

@ -79,8 +79,8 @@ ensure_logfile(Name, FD, Inode, Buffer) ->
{ok, {FD, Inode, FInfo#file_info.size}}; {ok, {FD, Inode, FInfo#file_info.size}};
false -> false ->
%% delayed write can cause file:close not to do a close %% delayed write can cause file:close not to do a close
file:close(FD),
file:close(FD),
_ = file:close(FD),
_ = file:close(FD),
case open_logfile(Name, Buffer) of case open_logfile(Name, Buffer) of
{ok, {FD2, Inode3, Size}} -> {ok, {FD2, Inode3, Size}} ->
%% inode changed, file was probably moved and %% inode changed, file was probably moved and
@ -92,8 +92,8 @@ ensure_logfile(Name, FD, Inode, Buffer) ->
end; end;
_ -> _ ->
%% delayed write can cause file:close not to do a close %% delayed write can cause file:close not to do a close
file:close(FD),
file:close(FD),
_ = file:close(FD),
_ = file:close(FD),
case open_logfile(Name, Buffer) of case open_logfile(Name, Buffer) of
{ok, {FD2, Inode3, Size}} -> {ok, {FD2, Inode3, Size}} ->
%% file was removed %% file was removed
@ -117,13 +117,15 @@ maybe_utc({Date, {H, M, S, Ms}}) ->
{Date1, {H1, M1, S1, Ms}} {Date1, {H1, M1, S1, Ms}}
end. end.
%% renames and deletes failing are OK
rotate_logfile(File, 0) -> rotate_logfile(File, 0) ->
file:delete(File);
_ = file:delete(File),
ok;
rotate_logfile(File, 1) -> rotate_logfile(File, 1) ->
file:rename(File, File++".0"),
_ = file:rename(File, File++".0"),
rotate_logfile(File, 0); rotate_logfile(File, 0);
rotate_logfile(File, Count) -> rotate_logfile(File, Count) ->
file:rename(File ++ "." ++ integer_to_list(Count - 2), File ++ "." ++
_ =file:rename(File ++ "." ++ integer_to_list(Count - 2), File ++ "." ++
integer_to_list(Count - 1)), integer_to_list(Count - 1)),
rotate_logfile(File, Count - 1). rotate_logfile(File, Count - 1).

Loading…
Cancel
Save