[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: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665342 13f79535-47bb-0310-9956-ffa450edef68
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 @@
%
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 @@
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 @@
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,
+ 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,
+ 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,
+ 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,
+ 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) -> %%
-%% []. %%
+ 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 @@
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);
+ 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),
+ ?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);
+ %% 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_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_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)
+ ?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),
+ Type == ?tType_STOP ->
+ ok;
- %% cpiro: this is here in original tprotocol.rb, but i think it's a bug
- % ?L0(readStructEnd),
- skip_struct_loop(This)
+ 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 @@
%%====================================================================
%%--------------------------------------------------------------------
%% @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 @@
%% @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 @@
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]);
+ %% 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]);
+ %% 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]);
+ %% 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])
+ %% 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,
+ {?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
+ 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_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 @@
%% {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 @@
%% @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 @@
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 @@
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.