From 96c1aa0041b368afceef0aef88e82a6c9f8e901d Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Fri, 31 Aug 2018 17:08:24 +0200 Subject: [logger] Remove encoding option from logger_formatter The encoding option was introduced in commit 270d909696a753af022df72a404c73f2895b4a02, to allow report callbacks to format according to a given encoding. There was, however, no connection between this encoding option, and the encoding of the device to which the logger handler was writing. Since a formatter is defined to return unicode:chardata(), and in order to avoid mismatch with the encoding of the device, the encoding option is now removed from the formatter. The handler itself must make sure that it does not write illegal data to its device. --- lib/kernel/doc/src/logger_chapter.xml | 11 +++-- lib/kernel/doc/src/logger_formatter.xml | 7 --- lib/kernel/src/logger.erl | 3 +- lib/kernel/src/logger_formatter.erl | 68 ++++++++++++----------------- lib/kernel/src/logger_h_common.erl | 20 ++++++--- lib/kernel/test/logger_disk_log_h_SUITE.erl | 2 +- lib/kernel/test/logger_std_h_SUITE.erl | 2 +- lib/stdlib/src/proc_lib.erl | 2 +- 8 files changed, 52 insertions(+), 63 deletions(-) diff --git a/lib/kernel/doc/src/logger_chapter.xml b/lib/kernel/doc/src/logger_chapter.xml index d58c4a4d42..26066d0777 100644 --- a/lib/kernel/doc/src/logger_chapter.xml +++ b/lib/kernel/doc/src/logger_chapter.xml @@ -208,12 +208,11 @@ coversion to a string:

fun((logger:report(),logger:report_cb_config()) -> unicode:chardata())
-

The fun must obey the encoding, depth - and chars_limit parameters provided in the second - argument, as the formatter can not do anything useful of these - parameters with the returned string. This variant is used when - the formatting of the report depends on the size and encoding - parameters.

+

The fun must obey the depth and chars_limit + parameters provided in the second argument, as the formatter + can not do anything useful of these parameters with the + returned string. This variant is used when the formatting of + the report depends on the size parameters.

Example, format string and arguments:

logger:error("The file does not exist: ~ts",[Filename])

Example, string:

diff --git a/lib/kernel/doc/src/logger_formatter.xml b/lib/kernel/doc/src/logger_formatter.xml index 5a060fd42b..24772fd6c4 100644 --- a/lib/kernel/doc/src/logger_formatter.xml +++ b/lib/kernel/doc/src/logger_formatter.xml @@ -82,13 +82,6 @@ in STDLIB.

Defaults to unlimited.

- encoding = - unicode:encoding() - -

This parameter must reflect the encoding of the device - that the handler prints to.

-

Defaults to utf8

- legacy_header = boolean()

If set to true a header field is added to diff --git a/lib/kernel/src/logger.erl b/lib/kernel/src/logger.erl index 7b1377b8ca..df96367d82 100644 --- a/lib/kernel/src/logger.erl +++ b/lib/kernel/src/logger.erl @@ -77,8 +77,7 @@ -type report() :: map() | [{atom(),term()}]. -type report_cb() :: fun((report()) -> {io:format(),[term()]}) | fun((report(),report_cb_config()) -> unicode:chardata()). --type report_cb_config() :: #{encoding := unicode:encoding(), - depth := pos_integer() | unlimited, +-type report_cb_config() :: #{depth := pos_integer() | unlimited, chars_limit := pos_integer() | unlimited}. -type msg_fun() :: fun((term()) -> {io:format(),[term()]} | report() | diff --git a/lib/kernel/src/logger_formatter.erl b/lib/kernel/src/logger_formatter.erl index b0d4adc14d..59aa6ec3af 100644 --- a/lib/kernel/src/logger_formatter.erl +++ b/lib/kernel/src/logger_formatter.erl @@ -28,7 +28,6 @@ %%% Types -type config() :: #{chars_limit => pos_integer() | unlimited, depth => pos_integer() | unlimited, - encoding => unicode:encoding(), legacy_header => boolean(), max_size => pos_integer() | unlimited, report_cb => logger:report_cb(), @@ -84,7 +83,7 @@ format(#{level:=Level,msg:=Msg0,meta:=Meta},Config0) true -> "" end, - truncate(B ++ MsgStr ++ A,maps:get(max_size,Config)). + truncate([B,MsgStr,A],maps:get(max_size,Config)). do_format(Level,Data,[level|Format],Config) -> [to_string(level,Level,Config)|do_format(Level,Data,Format,Config)]; @@ -147,7 +146,7 @@ printable_list(X) -> io_lib:printable_list(X). format_msg({string,Chardata},Meta,Config) -> - format_msg({s(Config),[Chardata]},Meta,Config); + format_msg({"~ts",[Chardata]},Meta,Config); format_msg({report,_}=Msg,Meta,#{report_cb:=Fun}=Config) when is_function(Fun,1); is_function(Fun,2) -> format_msg(Msg,Meta#{report_cb=>Fun},maps:remove(report_cb,Config)); @@ -166,13 +165,13 @@ format_msg({report,Report},#{report_cb:=Fun}=Meta,Config) when is_function(Fun,1 Meta,Config) end; format_msg({report,Report},#{report_cb:=Fun}=Meta,Config) when is_function(Fun,2) -> - try Fun(Report,maps:with([encoding,depth,chars_limit],Config)) of - String when ?IS_STRING(String) -> - try unicode:characters_to_list(String) + try Fun(Report,maps:with([depth,chars_limit],Config)) of + Chardata when ?IS_STRING(Chardata) -> + try chardata_to_list(Chardata) % already size limited by report_cb catch _:_ -> P = p(Config), format_msg({"REPORT_CB/2 ERROR: "++P++"; Returned: "++P, - [Report,String]},Meta,Config) + [Report,Chardata]},Meta,Config) end; Other -> P = p(Config), @@ -189,28 +188,27 @@ format_msg({report,Report},Meta,Config) -> Meta#{report_cb=>fun logger:format_report/1}, Config); format_msg(Msg,_Meta,#{depth:=Depth,chars_limit:=CharsLimit, - encoding:=Enc,single_line:=Single}) -> + single_line:=Single}) -> Opts = chars_limit_to_opts(CharsLimit), - format_msg(Msg, Depth, Opts, Enc, Single). + format_msg(Msg, Depth, Opts, Single). chars_limit_to_opts(unlimited) -> []; chars_limit_to_opts(CharsLimit) -> [{chars_limit,CharsLimit}]. -format_msg({Format0,Args},Depth,Opts,Enc,Single) -> +format_msg({Format0,Args},Depth,Opts,Single) -> try Format1 = io_lib:scan_format(Format0, Args), Format = reformat(Format1, Depth, Single), io_lib:build_text(Format,Opts) catch C:R:S -> - P = p(Enc,Single), + P = p(Single), FormatError = "FORMAT ERROR: "++P++" - "++P, case Format0 of FormatError -> %% already been here - avoid failing cyclically erlang:raise(C,R,S); _ -> - format_msg({FormatError,[Format0,Args]}, - Depth,Opts,Enc,Single) + format_msg({FormatError,[Format0,Args]},Depth,Opts,Single) end end. @@ -233,6 +231,14 @@ limit_depth(#{control_char:=C0, args:=Args}=M0, Depth) -> C = C0 - ($a - $A), %To uppercase. M0#{control_char:=C,args:=Args++[Depth]}. +chardata_to_list(Chardata) -> + case unicode:characters_to_list(Chardata,unicode) of + List when is_list(List) -> + List; + Error -> + throw(Error) + end. + truncate(String,unlimited) -> String; truncate(String,Size) -> @@ -278,12 +284,11 @@ maybe_add_legacy_header(Level, #{time:=Timestamp}=Meta, #{legacy_header:=true}=Config) -> #{title:=Title}=MyMeta = add_legacy_title(Level,Meta,Config), - {{Y,Mo,D},{H,Mi,Sec},Micro,UtcStr} = + {{Y,Mo,D},{H,Mi,S},Micro,UtcStr} = timestamp_to_datetimemicro(Timestamp,Config), - S = s(Config), Header = - io_lib:format("="++S++"==== ~w-~s-~4w::~2..0w:~2..0w:~2..0w.~6..0w ~s===", - [Title,D,month(Mo),Y,H,Mi,Sec,Micro,UtcStr]), + io_lib:format("=~ts==== ~w-~s-~4w::~2..0w:~2..0w:~2..0w.~6..0w ~s===", + [Title,D,month(Mo),Y,H,Mi,S,Micro,UtcStr]), Meta#{?MODULE=>MyMeta#{header=>Header}}; maybe_add_legacy_header(_,Meta,_) -> Meta. @@ -324,7 +329,6 @@ month(12) -> "Dec". add_default_config(Config0) -> Default = #{chars_limit=>unlimited, - encoding=>utf8, error_logger_notice_header=>info, legacy_header=>false, single_line=>true, @@ -502,25 +506,9 @@ check_timezone(Tz) -> error end. -p(#{encoding:=Enc, single_line:=Single}) -> - p(Enc,Single). - -p(Enc,Single) -> - "~"++p_width(Single)++p_char(Enc). - -p_width(true) -> - "0"; -p_width(false) -> - "". - -p_char(latin1) -> - "p"; -p_char(_) -> - "tp". - -s(#{encoding:=Enc}) -> - s(Enc); -s(latin1) -> - "~s"; -s(_) -> - "~ts". +p(#{single_line:=Single}) -> + p(Single); +p(true) -> + "~0tp"; +p(false) -> + "~tp". diff --git a/lib/kernel/src/logger_h_common.erl b/lib/kernel/src/logger_h_common.erl index 854e5479b9..2818a460f1 100644 --- a/lib/kernel/src/logger_h_common.erl +++ b/lib/kernel/src/logger_h_common.erl @@ -40,7 +40,7 @@ info_notify/1]). %%%----------------------------------------------------------------- -%%% Covert log data on any form to binary +%%% Convert log data on any form to binary -spec log_to_binary(LogEvent,Config) -> LogString when LogEvent :: logger:log_event(), Config :: logger:handler_config(), @@ -57,13 +57,14 @@ do_log_to_binary(Log,Config) -> {Formatter,FormatterConfig} = maps:get(formatter,Config,{?DEFAULT_FORMATTER,?DEFAULT_FORMAT_CONFIG}), String = try_format(Log,Formatter,FormatterConfig), - try unicode:characters_to_binary(String) - catch _:_ -> + try string_to_binary(String) + catch C2:R2:S2 -> ?LOG_INTERNAL(debug,[{formatter_error,Formatter}, {config,FormatterConfig}, {log_event,Log}, - {bad_return_value,String}]), - <<"FORMATTER ERROR: bad_return_value">> + {bad_return_value,String}, + {catched,{C2,R2,S2}}]), + <<"FORMATTER ERROR: bad return value">> end. try_format(Log,Formatter,FormatterConfig) -> @@ -85,6 +86,15 @@ try_format(Log,Formatter,FormatterConfig) -> end end. +string_to_binary(String) -> + case unicode:characters_to_binary(String) of + Binary when is_binary(Binary) -> + Binary; + Error -> + throw(Error) + end. + + %%%----------------------------------------------------------------- %%% Check that the configuration term is valid check_common_config({mode_tab,_Tid}) -> diff --git a/lib/kernel/test/logger_disk_log_h_SUITE.erl b/lib/kernel/test/logger_disk_log_h_SUITE.erl index a4b15c841b..29624b16c2 100644 --- a/lib/kernel/test/logger_disk_log_h_SUITE.erl +++ b/lib/kernel/test/logger_disk_log_h_SUITE.erl @@ -367,7 +367,7 @@ formatter_fail(Config) -> ok = logger:set_handler_config(Name,formatter,{?MODULE,bad_return}), logger:notice(?msg,?domain), try_match_file(?log_no(LogFile,1), - escape(Got3)++"FORMATTER ERROR: bad_return_value", + escape(Got3)++"FORMATTER ERROR: bad return value", 5000), %% Check that handler is still alive and was never dead diff --git a/lib/kernel/test/logger_std_h_SUITE.erl b/lib/kernel/test/logger_std_h_SUITE.erl index 0930cd4211..3426567bbf 100644 --- a/lib/kernel/test/logger_std_h_SUITE.erl +++ b/lib/kernel/test/logger_std_h_SUITE.erl @@ -280,7 +280,7 @@ formatter_fail(Config) -> ok = logger:set_handler_config(?MODULE,formatter,{?MODULE,bad_return}), logger:notice(?msg,?domain), try_match_file(Log, - escape(Got3)++"FORMATTER ERROR: bad_return_value", + escape(Got3)++"FORMATTER ERROR: bad return value", 5000), %% Check that handler is still alive and was never dead diff --git a/lib/stdlib/src/proc_lib.erl b/lib/stdlib/src/proc_lib.erl index d07c62500b..d5c92c9a37 100644 --- a/lib/stdlib/src/proc_lib.erl +++ b/lib/stdlib/src/proc_lib.erl @@ -757,7 +757,7 @@ check(Res) -> Res. report_cb(#{label:={proc_lib,crash}, report:=CrashReport}, Extra) -> Default = #{chars_limit => unlimited, depth => unlimited, - encoding => latin1}, + encoding => utf8}, do_format(CrashReport, maps:merge(Default,Extra)). -spec format(CrashReport) -> string() when -- cgit v1.2.1