From 0af738482ce4437673d9ef274f25a2d56b8e3059 Mon Sep 17 00:00:00 2001 From: "(no author)" <(no author)@41a61cd8-c433-0410-bb1c-e256eeef9e11> Date: Thu, 15 Nov 2007 06:27:33 +0000 Subject: [PATCH] r1298@opsdev009 (orig r69998): cpiro | 2007-11-14 22:26:30 -0800 [thrift] change up Erlang crash reports, cleanups in t{,Binary}Protocol Summary: crash reports aren't too important so don't be so verbose -- all the pertinent info shows up elsewhere anyway. refactor while we're at it. Reviewed By: eletuchy Test Plan: ok Revert Plan: ok git-svn-id: http://svn.facebook.com/svnroot/thrift/trunk@670 41a61cd8-c433-0410-bb1c-e256eeef9e11 --- lib/erl/src/protocol/tBinaryProtocol.erl | 50 +++---- lib/erl/src/protocol/tProtocol.erl | 178 +++++++++------------- lib/erl/src/thrift_logger.erl | 250 +++++++++++++++---------------- 3 files changed, 212 insertions(+), 266 deletions(-) diff --git a/lib/erl/src/protocol/tBinaryProtocol.erl b/lib/erl/src/protocol/tBinaryProtocol.erl index 9430e39..207f305 100644 --- a/lib/erl/src/protocol/tBinaryProtocol.erl +++ b/lib/erl/src/protocol/tBinaryProtocol.erl @@ -1,6 +1,6 @@ %%% Copyright (c) 2007- Facebook %%% Distributed under the Thrift Software License -%%% +%%% %%% See accompanying file LICENSE or visit the Thrift site at: %%% http://developers.facebook.com/thrift/ @@ -25,16 +25,16 @@ writeListBegin/3, writeSetBegin/3, - writeBool/2, writeByte/2, writeI16/2, writeI32/2, - writeI64/2, writeDouble/2, writeString/2, + writeBool/2, writeByte/2, writeI16/2, writeI32/2, + writeI64/2, writeDouble/2, writeString/2, - readMessageBegin/1, - readFieldBegin/1, - readMapBegin/1, - readListBegin/1, - readSetBegin/1, + readMessageBegin/1, + readFieldBegin/1, + readMapBegin/1, + readListBegin/1, + readSetBegin/1, - readBool/1, readByte/1, readI16/1, readI32/1, + readBool/1, readByte/1, readI16/1, readI32/1, readI64/1, readDouble/1, readString/1 ]). @@ -44,11 +44,11 @@ %%% ?DEFINE_ATTR(super). - + %%% %%% behavior callbacks %%% - + %%% super() -> SuperModule = atom() %%% | none @@ -106,9 +106,9 @@ writeSetBegin(This, Etype, Size) -> % writeBool(This, Bool) -> - case Bool of - true -> ?L1(writeByte, 1); - false -> ?L1(writeByte, 0) + case Bool of + true -> ?L1(writeByte, 1); + false -> ?L1(writeByte, 0) end. writeByte(This, Byte) -> @@ -140,11 +140,11 @@ writeString(This, Str) -> readMessageBegin(This) -> Version = ?L0(readI32), - if - (Version band ?VERSION_MASK) /= ?VERSION_1 -> - throw(tProtocolException:new(?tProtocolException_BAD_VERSION, - "Missing version identifier")); - true -> ok + if + (Version band ?VERSION_MASK) /= ?VERSION_1 -> + throw(tProtocolException:new(?tProtocolException_BAD_VERSION, + "Missing version identifier")); + true -> ok end, Type = Version band 16#000000ff, Name = ?L0(readString), @@ -153,12 +153,12 @@ readMessageBegin(This) -> readFieldBegin(This) -> Type = ?L0(readByte), - case Type of - ?tType_STOP -> - { nil, Type, 0 }; % WATCH - _ -> - Id = ?L0(readI16), - { nil, Type, Id } + case Type of + ?tType_STOP -> + { nil, Type, 0 }; % WATCH + _ -> + Id = ?L0(readI16), + { nil, Type, Id } end. readMapBegin(This) -> diff --git a/lib/erl/src/protocol/tProtocol.erl b/lib/erl/src/protocol/tProtocol.erl index 8900c5d..4ef67b8 100644 --- a/lib/erl/src/protocol/tProtocol.erl +++ b/lib/erl/src/protocol/tProtocol.erl @@ -1,6 +1,6 @@ %%% Copyright (c) 2007- Facebook %%% Distributed under the Thrift Software License -%%% +%%% %%% See accompanying file LICENSE or visit the Thrift site at: %%% http://developers.facebook.com/thrift/ @@ -15,72 +15,30 @@ -export([attr/4, super/0, inspect/1]). -%% -export([interface/1]). %% - -export([ - new/1, - skip/2, - - writeMessageBegin/4, writeMessageEnd/1, - writeStructBegin/2, writeStructEnd/1, - writeFieldBegin/4, writeFieldEnd/1, writeFieldStop/1, - writeMapBegin/4, writeMapEnd/1, - writeListBegin/3, writeListEnd/1, - writeSetBegin/3, writeSetEnd/1, - - writeBool/2, writeByte/2, writeI16/2, writeI32/2, - writeI64/2, writeDouble/2, writeString/2, - - readMessageBegin/1, readMessageEnd/1, - readStructBegin/1, readStructEnd/1, - readFieldBegin/1, readFieldEnd/1, - readMapBegin/1, readMapEnd/1, - readListBegin/1, readListEnd/1, - readSetBegin/1, readSetEnd/1, - - readBool/1, readByte/1, readI16/1, readI32/1, - readI64/1, readDouble/1, readString/1 -]). - -%%% -%%% server interface -%%% - -%% %%% modules we can instantiate from the server %% -%% interface(subclasses) -> %% -%% [ %% -%% tBinaryProtocol %% -%% ]; %% -%% %% -%% %%% synchronous calls to pass %% -%% interface(call) -> %% -%% [ %% -%% skip, %% -%% %% -%% writeMessageBegin, writeMessageEnd, %% -%% writeStructBegin, writeStructEnd, %% -%% writeFieldBegin, writeFieldEnd, writeFieldStop, %% -%% writeMapBegin, writeMapEnd, %% -%% writeListBegin, writeListEnd, %% -%% writeSetBegin, writeSetEnd, %% -%% %% -%% writeBool, writeByte, writeI16, writeI32, %% -%% writeI64, writeDouble, writeString, %% -%% %% -%% readMessageBegin, readMessageEnd, %% -%% readStructBegin, readStructEnd, %% -%% readFieldBegin, readFieldEnd, %% -%% readMapBegin, readMapEnd, %% -%% readListBegin, readListEnd, %% -%% readSetBegin, readSetEnd, %% -%% %% -%% readBool, readByte, readI16, readI32, %% -%% readI64, readDouble, readString %% -%% ]; %% -%% %% -%% %%% asynchronous casts to pass %% -%% interface(cast) -> %% -%% []. %% + new/1, + skip/2, + + writeMessageBegin/4, writeMessageEnd/1, + writeStructBegin/2, writeStructEnd/1, + writeFieldBegin/4, writeFieldEnd/1, writeFieldStop/1, + writeMapBegin/4, writeMapEnd/1, + writeListBegin/3, writeListEnd/1, + writeSetBegin/3, writeSetEnd/1, + + writeBool/2, writeByte/2, writeI16/2, writeI32/2, + writeI64/2, writeDouble/2, writeString/2, + + readMessageBegin/1, readMessageEnd/1, + readStructBegin/1, readStructEnd/1, + readFieldBegin/1, readFieldEnd/1, + readMapBegin/1, readMapEnd/1, + readListBegin/1, readListEnd/1, + readSetBegin/1, readSetEnd/1, + + readBool/1, readByte/1, readI16/1, readI32/1, + readI64/1, readDouble/1, readString/1 + ]). %%% %%% define attributes @@ -88,11 +46,11 @@ %%% ?DEFINE_ATTR(trans). - + %%% %%% behavior callbacks %%% - + %%% super() -> SuperModule = atom() %%% | none @@ -159,54 +117,54 @@ readDouble(_This) -> ok. readString(_This) -> ok. skip(This, Type) -> - case Type of - ?tType_STOP -> nil; % WATCH - ?tType_BOOL -> ?L0(readBool); - ?tType_BYTE -> ?L0(readByte); - ?tType_I16 -> ?L0(readI16); - ?tType_I32 -> ?L0(readI32); - ?tType_I64 -> ?L0(readI64); - ?tType_DOUBLE -> ?L0(readDouble); - ?tType_STRING -> ?L0(readString); - - ?tType_STRUCT -> - ?L0(readStructBegin), - skip_struct_loop(This), - - %% cpiro: this isn't here in the original tprotocol.rb, but i think it's a bug - ?L0(readStructEnd); - - ?tType_MAP -> - {Ktype, Vtype, Size} = ?L0(readMapBegin), - skip_map_repeat(This, Ktype, Vtype, Size), - ?L0(readMapEnd); - - ?tType_SET -> - {Etype, Size} = ?L0(readSetBegin), - skip_set_repeat(This, Etype, Size), - ?L0(readSetEnd); - - ?tType_LIST -> - {Etype, Size} = ?L0(readListBegin), - skip_set_repeat(This, Etype, Size), % [sic] skipping same as for SET - ?L0(readListEnd) + case Type of + ?tType_STOP -> nil; % WATCH + ?tType_BOOL -> ?L0(readBool); + ?tType_BYTE -> ?L0(readByte); + ?tType_I16 -> ?L0(readI16); + ?tType_I32 -> ?L0(readI32); + ?tType_I64 -> ?L0(readI64); + ?tType_DOUBLE -> ?L0(readDouble); + ?tType_STRING -> ?L0(readString); + + ?tType_STRUCT -> + ?L0(readStructBegin), + skip_struct_loop(This), + + %% cpiro: this isn't here in the original tprotocol.rb, but i think it's a bug + ?L0(readStructEnd); + + ?tType_MAP -> + {Ktype, Vtype, Size} = ?L0(readMapBegin), + skip_map_repeat(This, Ktype, Vtype, Size), + ?L0(readMapEnd); + + ?tType_SET -> + {Etype, Size} = ?L0(readSetBegin), + skip_set_repeat(This, Etype, Size), + ?L0(readSetEnd); + + ?tType_LIST -> + {Etype, Size} = ?L0(readListBegin), + skip_set_repeat(This, Etype, Size), % [sic] skipping same as for SET + ?L0(readListEnd) end. skip_struct_loop(This) -> { _Name, Type, _Id } = ?L0(readFieldBegin), if - Type == ?tType_STOP -> - ok; - - true -> - ?L1(skip, Type), - ?L0(readFieldEnd), - - %% cpiro: this is here in original tprotocol.rb, but i think it's a bug - % ?L0(readStructEnd), - skip_struct_loop(This) + Type == ?tType_STOP -> + ok; + + true -> + ?L1(skip, Type), + ?L0(readFieldEnd), + + %% cpiro: this is here in original tprotocol.rb, but i think it's a bug + %% ?L0(readStructEnd), + skip_struct_loop(This) end. - + skip_map_repeat(This, Ktype, Vtype, Times) -> ?L1(skip, Ktype), ?L1(skip, Vtype), diff --git a/lib/erl/src/thrift_logger.erl b/lib/erl/src/thrift_logger.erl index e266ef4..31a71b9 100644 --- a/lib/erl/src/thrift_logger.erl +++ b/lib/erl/src/thrift_logger.erl @@ -1,6 +1,6 @@ %%% Copyright (c) 2007- Facebook %%% Distributed under the Thrift Software License -%%% +%%% %%% See accompanying file LICENSE or visit the Thrift site at: %%% http://developers.facebook.com/thrift/ @@ -18,13 +18,19 @@ -include("transport/tTransportException.hrl"). %% gen_event callbacks --export([init/1, handle_event/2, handle_call/2, +-export([init/1, handle_event/2, handle_call/2, handle_info/2, terminate/2, code_change/3]). -export([install/0]). +%% + -record(state, {}). +-define(GS_TERM_FORMAT, "** Generic server ~p terminating \n** Last message in was ~p~n** When Server state == ~p~n** Reason for termination == ~n** ~p~n"). + +%% + %% ensure the regular logger is out and ours is in install() -> %% remove loggers @@ -47,11 +53,11 @@ install() -> %%==================================================================== %%-------------------------------------------------------------------- %% @spec init(Args) -> {ok, State}. -%% -%% @doc +%% +%% @doc %% Whenever a new event handler is added to an event manager, %% this function is called to initialize the event handler. -%% @end +%% @end %%-------------------------------------------------------------------- init([]) -> State = #state{}, @@ -61,21 +67,21 @@ init([]) -> %% @spec handle_event(Event, State) -> {ok, State} | %% {swap_handler, Args1, State1, Mod2, Args2} | %% remove_handler. -%% -%% @doc +%% +%% @doc %% Whenever an event manager receives an event sent using %% gen_event:notify/2 or gen_event:sync_notify/2, this function is called for -%% each installed event handler to handle the event. -%% @end +%% each installed event handler to handle the event. +%% @end %%-------------------------------------------------------------------- handle_event2(Symbol, Pid, Type, Message, State) -> % Message must be a string {ok, MessageSafe, NL} = regexp:gsub(Message, "[\n]+", " "), % collapse whitespace to one space Type1 = - case Type of - "" -> ""; - _ -> sformat("~p ", [Type]) - end, + case Type of + "" -> ""; + _ -> sformat("~p ", [Type]) + end, Banner = case config(show_pid) of @@ -91,116 +97,93 @@ handle_event2(Symbol, Pid, Type, Message, State) -> % Message must be a string OutputSafe = sformat("~s", [MessageSafe]), Length = - case (length(OutputSafe) + BannerLen) < config(term_width) of - true -> short; - false -> long - end, + case (length(OutputSafe) + BannerLen) < config(term_width) of + true -> short; + false -> long + end, OneLine = - case NL == 0 of - true -> oneliner; - false -> multiline - end, + case NL == 0 of + true -> oneliner; + false -> multiline + end, case { config(force_one_line), Length, OneLine } of - %% one line and short ... print as is - {_, short, oneliner} -> - format("~s~s~n", [Banner, OutputSafe]); - - %% too long ... squash to one - {true, long, _} -> - O = Banner ++ OutputSafe, - Format = sformat("~~~ps >~n", [config(term_width)-2]), % e.g. "~80s >~n" - format(Format, [O]); - - %% short but multiline... collapse to one - {true, short, multiline} -> - format("~s~s~n", [Banner, OutputSafe]); - - %% just print it - _ -> - format("~s~n~s~n~n", [Banner, Output]) + %% one line and short ... print as is + {_, short, oneliner} -> + format("~s~s~n", [Banner, OutputSafe]); + + %% too long ... squash to one + {true, long, _} -> + O = Banner ++ OutputSafe, + Format = sformat("~~~ps >~n", [config(term_width)-2]), % e.g. "~80s >~n" + format(Format, [O]); + + %% short but multiline... collapse to one + {true, short, multiline} -> + format("~s~s~n", [Banner, OutputSafe]); + + %% just print it + _ -> + format("~s~n~s~n~n", [Banner, Output]) end. %% --define(GS_TERM_FORMAT, "** Generic server ~p terminating \n** Last message in was ~p~n** When Server state == ~p~n** Reason for termination == ~n** ~p~n"). handle_event1({What, _Gleader, {Ref, Format, Data}}, State) when is_list(Format) -> - Symbol = case What of - error -> "!!"; - warning_msg -> "**"; - info_msg -> ".."; - _Else -> "??" - end, + Symbol = symbol(What), case {Format, Data} of - {?GS_TERM_FORMAT, [Ref, LastMessage, Obj, Reason]} -> - %% TODO: move as much logic as possible out of thrift_logger - Ignore = - begin - is_tuple(Reason) andalso - size(Reason) >= 1 andalso element(1, Reason) == timeout - end - orelse - begin - case thrift_utils:unnest_record(Reason, tTransportException) of - error -> false; - {ok, TTE} -> - oop:get(TTE, type) == ?tTransportException_NOT_OPEN andalso - oop:get(TTE, message) == "in tSocket:read/2: gen_tcp:recv" - end - end, - - case Ignore of - true -> - ok; - false -> - Format1 = "** gen_server terminating in message ~p~n** State = ~s~n** Reason = ~s~n", - Message = sformat(Format1, [LastMessage, oop:inspect(Obj), oop:inspect(Reason)]), %% TODO(cpiro): hope Reason is an object? - handle_event2(Symbol, Ref, "", Message, State) - end; - {?GS_TERM_FORMAT, _Dta} -> - Message = sformat("DATA DIDN'T MATCH: ~p~n", [Data]) ++ sformat(Format, Data), - handle_event2(Symbol, Ref, "", Message, State); - {_, _} -> - case lists:member(Format, config(omit_fmt)) of - false -> - Message = sformat(Format, Data), - handle_event2(Symbol, Ref, "", Message, State); - true -> - ok - end + {?GS_TERM_FORMAT, [Ref, LastMessage, Obj, Reason]} -> + %% TODO: move as much logic as possible out of thrift_logger + Ignore = + begin + is_tuple(Reason) andalso + size(Reason) >= 1 andalso element(1, Reason) == timeout + end + orelse + begin + case thrift_utils:unnest_record(Reason, tTransportException) of + error -> false; + {ok, TTE} -> + oop:get(TTE, type) == ?tTransportException_NOT_OPEN andalso + oop:get(TTE, message) == "in tSocket:read/2: gen_tcp:recv" + end + end, + + case Ignore of + true -> + ok; + false -> + Format1 = "** gen_server terminating in message ~p~n** State = ~s~n** Reason = ~s~n", + Message = sformat(Format1, [LastMessage, oop:inspect(Obj), oop:inspect(Reason)]), %% TODO(cpiro): hope Reason is an object? + handle_event2(Symbol, Ref, "", Message, State) + end; + {?GS_TERM_FORMAT, _Dta} -> + Message = sformat("DATA DIDN'T MATCH: ~p~n", [Data]) ++ sformat(Format, Data), + handle_event2(Symbol, Ref, "", Message, State); + {_, _} -> + case lists:member(Format, config(omit_fmt)) of + false -> + Message = sformat(Format, Data), + handle_event2(Symbol, Ref, "", Message, State); + true -> + ok + end end, {ok, State}; handle_event1({What, _Gleader, {Pid, Type, Report}}, State) -> - Symbol = case What of - error_report -> "!!"; - warning_report -> "**"; - info_report -> ".."; - _Else -> "??" - end, + Symbol = symbol(What), case Type of crash_report -> - case do_print_crash_report(Report) of - true -> - io:format("~~~~ crash report: '~p'~n", [Report]); - false -> - ok - end; -%% crash_report -> -%% [Cruft|_] = Report, -%% {value, {_, Reason}} = lists:keysearch(error_info, 1, Cruft), -%% {value, {_, {_, _, [_,_,_,_, Obj, []]}}} = lists:keysearch(initial_call, 1, Cruft), -%% sformat("state == ~s~nreason ==~s", [oop:inspect(Obj), oop:inspect(Reason)]), -%% ok; + print_crash_report(Report); progress -> - ok; - - _ -> - Message = sformat("|| ~s", [oop:inspect(Report)]), - handle_event2(Symbol, Pid, Type, Message, State) + ok; + _ -> + Message = sformat("|| ~s", [oop:inspect(Report)]), + handle_event2(Symbol, Pid, Type, Message, State) end, {ok, State}; @@ -210,12 +193,12 @@ handle_event1(_Event, State) -> handle_event(Event, State) -> try - handle_event1(Event, State) + handle_event1(Event, State) catch - _:E -> - format("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~n error logger error:~n ~p~n Event = ~p~n State = ~p~n ~p~n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~n", - [E, Event, State, erlang:get_stacktrace()]), - {ok, State} + _:E -> + format("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~n error logger error:~n ~p~n Event = ~p~n State = ~p~n ~p~n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~n", + [E, Event, State, erlang:get_stacktrace()]), + {ok, State} end. %%-------------------------------------------------------------------- @@ -223,12 +206,12 @@ handle_event(Event, State) -> %% {swap_handler, Reply, Args1, State1, %% Mod2, Args2} | %% {remove_handler, Reply}. -%% -%% @doc +%% +%% @doc %% Whenever an event manager receives a request sent using -%% gen_event:call/3,4, this function is called for the specified event +%% gen_event:call/3,4, this function is called for the specified event %% handler to handle the request. -%% @end +%% @end %%-------------------------------------------------------------------- handle_call(_Request, State) -> Reply = ok, @@ -238,24 +221,24 @@ handle_call(_Request, State) -> %% @spec handle_info(Info, State) -> {ok, State} | %% {swap_handler, Args1, State1, Mod2, Args2} | %% remove_handler. -%% -%% @doc +%% +%% @doc %% This function is called for each installed event handler when %% an event manager receives any other message than an event or a synchronous %% request (or a system message). -%% @end +%% @end %%-------------------------------------------------------------------- handle_info(_Info, State) -> {ok, State}. %%-------------------------------------------------------------------- %% @spec terminate(Reason, State) -> void(). -%% -%% @doc +%% +%% @doc %% Whenever an event handler is deleted from an event manager, -%% this function is called. It should be the opposite of Module:init/1 and -%% do any necessary cleaning up. -%% @end +%% this function is called. It should be the opposite of Module:init/1 and +%% do any necessary cleaning up. +%% @end %%-------------------------------------------------------------------- terminate(normal, _State) -> ok; @@ -264,11 +247,11 @@ terminate(Reason, _State) -> ok. %%-------------------------------------------------------------------- -%% @spec code_change(OldVsn, State, Extra) -> {ok, NewState}. -%% -%% @doc +%% @spec code_change(OldVsn, State, Extra) -> {ok, NewState}. +%% +%% @doc %% Convert process state when code is changed -%% @end +%% @end %%-------------------------------------------------------------------- code_change(_OldVsn, State, _Extra) -> {ok, State}. @@ -288,15 +271,20 @@ sformat(Format, Data) -> config(Item) -> thrift:config(Item). -do_print_crash_report(Report) -> +symbol(error_report) -> "!!"; +symbol(warning_report) -> "**"; +symbol(info_report) -> ".."; +symbol(_Else) -> "??". + +print_crash_report(Report) -> case Report of - [[_,_,{error_info, XXX}|_] | _] -> - case thrift_utils:first_item(XXX) of + [[_,_,{error_info, XX}|_] | _] -> + case thrift_utils:first_item(XX) of tTransportException -> - false; + ok; _ -> - true + io:format("~~~~ crash report: ~P~n", [XX, 2]) end; _ -> - true + io:format("~~~~ crash report (?): ~p~n", [Report]) end. -- 2.11.4.GIT