[thrift] gut Erlang exception handling
Summary: * move type field to tException from subclasses
* add backtrace to tException
* add oop:is_a
* on exit, wrap exceptions in {thrift_exception, E} ... otherwise can't distinguish e.g. exit:{{tBinProtException, {tException, ...}}, Stack} vs. exit:{tBinProtException, {tException, ...} -- I hate erlang
* all throws/exits to tException:throw which does the wrapping described above
Reviewed By: eletuchy
Test Plan: been using this code on my live server ^_^
Revert: OK
git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665350 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/lib/erl/include/oop.hrl b/lib/erl/include/oop.hrl
index 84d81e1..66d1a0c 100644
--- a/lib/erl/include/oop.hrl
+++ b/lib/erl/include/oop.hrl
@@ -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/
@@ -16,10 +16,10 @@
%%% convenience for implementing inspect/1
%%% e.g. -> "foo=5"
-define(FORMAT_ATTR(Attr),
- io_lib:write_atom(Attr) ++ "=" ++ io_lib:print(?ATTR(Attr))
+ io_lib:write_atom(Attr) ++ "=" ++ io_lib:print(?ATTR(Attr))
).
--define(ATTR_DUMMY,
- attr(dummy, dummy, dummy, dummy) ->
- throw(dummy_attr_used)
+-define(ATTR_DUMMY,
+ attr(dummy, dummy, dummy, dummy) ->
+ exit(dummy_attr_used)
).
diff --git a/lib/erl/include/protocol/tProtocolException.hrl b/lib/erl/include/protocol/tProtocolException.hrl
index 2de72d7..9d2f31a 100644
--- a/lib/erl/include/protocol/tProtocolException.hrl
+++ b/lib/erl/include/protocol/tProtocolException.hrl
@@ -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/
@@ -10,6 +10,6 @@
-define(tProtocolException_SIZE_LIMIT, 3).
-define(tProtocolException_BAD_VERSION, 4).
--record(tProtocolException, {super, type}).
+-record(tProtocolException, {super}).
diff --git a/lib/erl/include/tApplicationException.hrl b/lib/erl/include/tApplicationException.hrl
index db7ec2f..5e2b515 100644
--- a/lib/erl/include/tApplicationException.hrl
+++ b/lib/erl/include/tApplicationException.hrl
@@ -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/
@@ -13,4 +13,4 @@
-define(tApplicationException_MISSING_RESULT, 5).
-define(tApplicationException_HANDLER_ERROR, 6).
--record(tApplicationException, {super, type}).
+-record(tApplicationException, {super}).
diff --git a/lib/erl/include/tException.hrl b/lib/erl/include/tException.hrl
index 808a474..896e8cb 100644
--- a/lib/erl/include/tException.hrl
+++ b/lib/erl/include/tException.hrl
@@ -1,7 +1,7 @@
%%% 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/
--record(tException, {message}).
+-record(tException, {message, type, backtrace}).
diff --git a/lib/erl/include/thrift_constants.hrl b/lib/erl/include/thrift_constants.hrl
index 8aad0a9..1948061 100644
--- a/lib/erl/include/thrift_constants.hrl
+++ b/lib/erl/include/thrift_constants.hrl
@@ -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/
diff --git a/lib/erl/include/thrift_macros.hrl b/lib/erl/include/thrift_macros.hrl
index 9b8a200..a1a6bad 100644
--- a/lib/erl/include/thrift_macros.hrl
+++ b/lib/erl/include/thrift_macros.hrl
@@ -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/
diff --git a/lib/erl/include/transport/tTransportException.hrl b/lib/erl/include/transport/tTransportException.hrl
index 1a60aad..0fcc99f 100644
--- a/lib/erl/include/transport/tTransportException.hrl
+++ b/lib/erl/include/transport/tTransportException.hrl
@@ -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/
@@ -10,4 +10,4 @@
-define(tTransportException_TIMED_OUT, 3).
-define(tTransportException_END_OF_FILE, 4).
--record(tTransportException, {super, type}).
+-record(tTransportException, {super}).
diff --git a/lib/erl/src/oop.erl b/lib/erl/src/oop.erl
index e4de147..418f0f2 100644
--- a/lib/erl/src/oop.erl
+++ b/lib/erl/src/oop.erl
@@ -14,7 +14,7 @@
-module(oop).
--export([start_new/2, get/2, set/3, call/2, call/3, inspect/1, class/1, is_object/1]).
+-export([start_new/2, get/2, set/3, call/2, call/3, inspect/1, class/1, is_object/1, is_a/2]).
-export([call1/3]). %% only for thrift_oop_server ... don't use it
-export([behaviour_info/1]).
@@ -65,7 +65,7 @@
{ok, V} -> V;
undef ->
case get_superobject(Obj) of
- { ok, Superobj } ->
+ {ok, Superobj} ->
Superobj1 = set(Superobj, Field, Value),
Module:attr(Obj, set, super, Superobj1);
undef ->
@@ -180,11 +180,30 @@
class(Obj) when is_tuple(Obj) ->
%% if it's an object its first element will be a class name, and it'll have super/0
case apply_if_defined(?CLASS(Obj), super, []) of
- {ok, V} -> V;
+ {ok, _} -> ?CLASS(Obj);
undef -> none
end;
class(_) -> none.
+%% is_a relationship
+is_a(Obj, Class) ->
+ %% ?INFO("is_a ~p ~p", [Obj, Class]),
+ case is_object(Obj) of
+ true ->
+ is_a1(Obj, Class);
+ false ->
+ false
+ end.
+is_a1(Obj, Class) when Class == ?CLASS(Obj) ->
+ true;
+is_a1(Obj, Class) ->
+ case get_superobject(Obj) of
+ undef ->
+ false;
+ {ok, SuperObj} ->
+ is_a1(SuperObj, Class)
+ end.
+
%% is the tuple/record an object?
%% is_object(Obj) = bool()
is_object(Obj) when is_tuple(Obj) ->
@@ -209,7 +228,7 @@
%% io:format("apply ~p ~p ~p~n", [M,F,A]),
{ok, apply(M, F, A)}
catch
- error:Kind when Kind == undef; Kind == function_clause ->
+ _:Kind when Kind == undef; Kind == function_clause ->
case erlang:get_stacktrace() of
%% the first stack call should match MFA when `apply' fails because the function is undefined
%% they won't match if the function is currently running and an error happens in the middle
diff --git a/lib/erl/src/protocol/tBinaryProtocol.erl b/lib/erl/src/protocol/tBinaryProtocol.erl
index 207f305..efcfd77 100644
--- a/lib/erl/src/protocol/tBinaryProtocol.erl
+++ b/lib/erl/src/protocol/tBinaryProtocol.erl
@@ -136,14 +136,13 @@
?L1(writeI32, length(Str)),
?R1(Trans, effectful_write, Str).
-%
+%%
readMessageBegin(This) ->
Version = ?L0(readI32),
if
(Version band ?VERSION_MASK) /= ?VERSION_1 ->
- throw(tProtocolException:new(?tProtocolException_BAD_VERSION,
- "Missing version identifier"));
+ tException:throw(tProtocolException, [?tProtocolException_BAD_VERSION, "Missing version identifier"]);
true -> ok
end,
Type = Version band 16#000000ff,
@@ -177,7 +176,7 @@
Size = ?L0(readI32),
{ Etype, Size }.
-%
+%%
readBool(This) ->
Byte = ?L0(readByte),
diff --git a/lib/erl/src/protocol/tProtocolException.erl b/lib/erl/src/protocol/tProtocolException.erl
index d926aff..84833a8 100644
--- a/lib/erl/src/protocol/tProtocolException.erl
+++ b/lib/erl/src/protocol/tProtocolException.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/
@@ -22,13 +22,12 @@
%%% 'super' is required unless ?MODULE is a base class
%%%
-?DEFINE_ATTR(super);
-?DEFINE_ATTR(type).
-
+?DEFINE_ATTR(super).
+
%%%
%%% behavior callbacks
%%%
-
+
%%% super() -> SuperModule = atom()
%%% | none
@@ -38,15 +37,15 @@
%%% inspect(This) -> string()
inspect(This) ->
- ?FORMAT_ATTR(type).
+ "".
%%%
%%% class methods
%%%
new(Type, Message) ->
- Super = (super()):new(Message),
- #?MODULE{super=Super, type=Type}.
+ Super = (super()):new(Type, Message),
+ #?MODULE{super=Super}.
new() ->
new(?tProtocolException_UNKNOWN, undefined).
diff --git a/lib/erl/src/server/tErlServer.erl b/lib/erl/src/server/tErlServer.erl
index 4e1f6fc..10ac2b2 100644
--- a/lib/erl/src/server/tErlServer.erl
+++ b/lib/erl/src/server/tErlServer.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/
@@ -16,7 +16,7 @@
-export([attr/4, super/0, inspect/1]).
--export([new/6, new/5, new/4, effectful_serve/1, effectful_new_acceptor/1, catches/3]).
+-export([new/6, new/5, new/4, effectful_serve/1, effectful_new_acceptor/1]).
%%%
%%% define attributes
@@ -27,11 +27,11 @@
?DEFINE_ATTR(acceptor);
?DEFINE_ATTR(listenSocket);
?DEFINE_ATTR(port).
-
+
%%%
%%% behavior callbacks
%%%
-
+
%%% super() -> SuperModule = atom()
%%% | none
@@ -67,20 +67,20 @@
Options = [binary, {packet, 0}, {active, false}],
%% listen
- case gen_tcp:listen(Port, Options) of
- {ok, ListenSocket} ->
- ?INFO("thrift server listening on port ~p", [Port]),
+ case gen_tcp:listen(Port, Options) of
+ {ok, ListenSocket} ->
+ ?INFO("thrift server listening on port ~p", [Port]),
- This1 = oop:set(This, listenSocket, ListenSocket),
+ This1 = oop:set(This, listenSocket, ListenSocket),
- %% spawn acceptor
- {_Acceptor, This2} = effectful_new_acceptor(This1),
+ %% spawn acceptor
+ {_Acceptor, This2} = effectful_new_acceptor(This1),
- {ok, This2};
+ {ok, This2};
- {error, eaddrinuse} ->
- ?ERROR("thrift couldn't bind port ~p, address in use", [Port]),
- {{error, eaddrinuse}, This} %% state before the accept
+ {error, eaddrinuse} ->
+ ?ERROR("thrift couldn't bind port ~p, address in use", [Port]),
+ {{error, eaddrinuse}, This} %% state before the accept
end.
effectful_new_acceptor(This) ->
@@ -100,18 +100,3 @@
This1 = oop:set(This, acceptor, Acceptor),
{Acceptor, This1}.
-
-catches(_This, _Pid, normal) ->
- ok.
-
-%% %% The current acceptor has died, wait a little and try again %%
-%% handle_info({'EXIT', Pid, _Abnormal}, #state{acceptor=Pid} = State) -> %%
-%% timer:sleep(2000), %%
-%% iserve_socket:start_link(self(), State#state.listen_socket, State#state.port), %%
-%% {noreply,State}; %%
-
-%% terminate(Reason, State) -> %%
-%% ?INFO( "Terminating error: ~p~n", [Reason]), % added %%
-%% gen_tcp:close(State#state.listen_socket), %%
-%% ok. %%
-%% %%
diff --git a/lib/erl/src/tApplicationException.erl b/lib/erl/src/tApplicationException.erl
index 568b9c9..d99b003 100644
--- a/lib/erl/src/tApplicationException.erl
+++ b/lib/erl/src/tApplicationException.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/
@@ -22,9 +22,8 @@
%%% 'super' is required unless ?MODULE is a base class
%%%
-?DEFINE_ATTR(super);
-?DEFINE_ATTR(type).
-
+?DEFINE_ATTR(super).
+
%%%
%%% behavior callbacks
%%%
@@ -38,15 +37,15 @@
%%% inspect(This) -> string()
inspect(This) ->
- ?FORMAT_ATTR(type).
+ "".
%%%
%%% class methods
%%%
new(Type, Message) ->
- Super = (super()):new(Message),
- #?MODULE{super=Super, type=Type}.
+ Super = (super()):new(Type, Message),
+ #?MODULE{super=Super}.
new() -> new(?tApplicationException_UNKNOWN, undefined).
new(Type) -> new(Type, undefined).
@@ -64,49 +63,49 @@
read_while_loop(This, Iprot) ->
{_Fname, Ftype, Fid} = ?R0(Iprot, readFieldBegin),
- if
- Ftype == ?tType_STOP ->
- ok;
+ if
+ Ftype == ?tType_STOP ->
+ ok;
- (Fid == 1) and (Ftype == ?tType_STRING) ->
- Message1 = ?R0(Iprot, readString),
- This1 = oop:set(This, message, Message1),
- ?R0(Iprot, readFieldEnd),
- read_while_loop(This1, Iprot);
+ (Fid == 1) and (Ftype == ?tType_STRING) ->
+ Message1 = ?R0(Iprot, readString),
+ This1 = oop:set(This, message, Message1),
+ ?R0(Iprot, readFieldEnd),
+ read_while_loop(This1, Iprot);
- Fid == 1 ->
- ?R0(Iprot, skip),
- ?R0(Iprot, readFieldEnd),
- read_while_loop(This, Iprot);
+ Fid == 1 ->
+ ?R0(Iprot, skip),
+ ?R0(Iprot, readFieldEnd),
+ read_while_loop(This, Iprot);
- (Fid == 2) and (Ftype == ?tType_I32) ->
- Type1 = ?R0(Iprot, readI32),
- This1 = oop:set(This, type, Type1),
- ?R0(Iprot, readFieldEnd),
- read_while_loop(This1, Iprot);
+ (Fid == 2) and (Ftype == ?tType_I32) ->
+ Type1 = ?R0(Iprot, readI32),
+ This1 = oop:set(This, type, Type1),
+ ?R0(Iprot, readFieldEnd),
+ read_while_loop(This1, Iprot);
- true ->
- ?R0(Iprot, skip),
- ?R0(Iprot, readFieldEnd),
- read_while_loop(This, Iprot)
+ true ->
+ ?R0(Iprot, skip),
+ ?R0(Iprot, readFieldEnd),
+ read_while_loop(This, Iprot)
end.
-write(This, Oprot) ->
+write(This, Oprot) ->
?R1(Oprot, writeStructBegin, "tApplicationException"),
Message = oop:get(This, message),
Type = oop:get(This, type),
- if Message /= undefined ->
- ?R3(Oprot, writeFieldBegin, "message", ?tType_STRING, 1),
- ?R1(Oprot, writeString, Message),
- ?R0(Oprot, writeFieldEnd);
+ if Message /= undefined ->
+ ?R3(Oprot, writeFieldBegin, "message", ?tType_STRING, 1),
+ ?R1(Oprot, writeString, Message),
+ ?R0(Oprot, writeFieldEnd);
true -> ok
end,
- if Type /= undefined ->
- ?R3(Oprot, writeFieldBegin, "type", ?tType_I32, 2),
- ?R1(Oprot, writeI32, Type),
- ?R0(Oprot, writeFieldEnd);
+ if Type /= undefined ->
+ ?R3(Oprot, writeFieldBegin, "type", ?tType_I32, 2),
+ ?R1(Oprot, writeI32, Type),
+ ?R0(Oprot, writeFieldEnd);
true -> ok
end,
diff --git a/lib/erl/src/tErlProcessor.erl b/lib/erl/src/tErlProcessor.erl
index a6d1073..2e88b6d 100644
--- a/lib/erl/src/tErlProcessor.erl
+++ b/lib/erl/src/tErlProcessor.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/
@@ -24,11 +24,11 @@
?DEFINE_ATTR(super);
?DEFINE_ATTR(generatedProcessor);
?DEFINE_ATTR(handler).
-
+
%%%
%%% behavior callbacks
%%%
-
+
%%% super() -> SuperModule = atom()
%%% | none
diff --git a/lib/erl/src/tException.erl b/lib/erl/src/tException.erl
index 0ec4c94..6dd3084 100644
--- a/lib/erl/src/tException.erl
+++ b/lib/erl/src/tException.erl
@@ -1,31 +1,36 @@
%%% 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/
-module(tException).
-include("oop.hrl").
+-include("thrift.hrl").
-include("tException.hrl").
-behavior(oop).
-export([attr/4, super/0, inspect/1]).
--export([new/1]).
+-export([new/2, add_backtrace_element/2, throw/2, inspect_with_backtrace/2, inspect_with_backtrace/3]).
+
+-export([read/1]).
%%%
%%% define attributes
%%% 'super' is required unless ?MODULE is a base class
%%%
-?DEFINE_ATTR(message).
-
+?DEFINE_ATTR(message);
+?DEFINE_ATTR(type);
+?DEFINE_ATTR(backtrace).
+
%%%
%%% behavior callbacks
%%%
-
+
%%% super() -> SuperModule = atom()
%%% | none
@@ -35,16 +40,54 @@
%%% inspect(This) -> string()
inspect(This) ->
- ?FORMAT_ATTR(message).
+ BT = ?ATTR(backtrace),
+ Depth =
+ if
+ is_list(BT) -> integer_to_list(length(BT));
+ true -> "?"
+ end,
+ ?FORMAT_ATTR(message) ++ ", " ++
+ ?FORMAT_ATTR(type) ++ ", "
+ " backtrace:" ++ Depth.
%%%
%%% class methods
%%%
-new(Message) ->
- #?MODULE{message=Message}.
+new(Type, Message) ->
+ #?MODULE{type=Type, message=Message, backtrace=[]}.
-%%%
-%%% instance methods
-%%%
+add_backtrace_element(E, Info) ->
+ BT = oop:get(E, backtrace),
+ E1 = oop:set(E, backtrace, [Info|BT]),
+ E1.
+throw(Class, Args) ->
+ E = apply(Class, new, Args),
+ exit({thrift_exception, E}).
+
+
+inspect_with_backtrace(E, Where, Info) ->
+ E1 = add_backtrace_element(E, Info),
+ inspect_with_backtrace(E1, Where).
+
+inspect_with_backtrace(E, Where) ->
+ thrift_utils:sformat("** ~s~n** ~s", [Where, oop:inspect(E)]) ++
+ case oop:get(E, backtrace) of
+ [] ->
+ "";
+ BT when is_list(BT) ->
+ thrift_utils:sformat("~n** trace = ~p", [lists:reverse(BT)]);
+ Else ->
+ thrift_utils:sformat("<ERROR BT NOT A LIST = ~p>", [Else])
+ end.
+
+read(E) ->
+ case oop:class(E) of
+ none ->
+ none;
+ Class ->
+ Type = oop:get(E, type),
+ BT = oop:get(E, backtrace),
+ {Class, Type, BT}
+ end.
diff --git a/lib/erl/src/thrift_logger.erl b/lib/erl/src/thrift_logger.erl
index 0d0b876..c8fb49a 100644
--- a/lib/erl/src/thrift_logger.erl
+++ b/lib/erl/src/thrift_logger.erl
@@ -8,14 +8,8 @@
-behaviour(gen_event).
-%% TODO(cpiro): either
-%% make exceptions know whether they need to be displayed
-%% or not exit with tExecptions for non-errors
-%% or "register" tExceptions with the logger (I LIKE!)
-%% ... we shouldn't need to build any specifics in here
-include("thrift.hrl").
-include("oop.hrl").
--include("transport/tTransportException.hrl").
%% gen_event callbacks
-export([init/1, handle_event/2, handle_call/2,
@@ -29,9 +23,10 @@
-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
+%%%
-%% ensure the regular logger is out and ours is in
install() ->
%% remove loggers
io:format("starting logger~n"),
@@ -48,32 +43,18 @@
ok.
-%%====================================================================
-%% gen_event callbacks
-%%====================================================================
-%%--------------------------------------------------------------------
-%% @spec init(Args) -> {ok, State}.
-%%
-%% @doc
-%% Whenever a new event handler is added to an event manager,
-%% this function is called to initialize the event handler.
-%% @end
-%%--------------------------------------------------------------------
+%%%
+%%% init
+%%%
+
init([]) ->
State = #state{},
{ok, State}.
-%%--------------------------------------------------------------------
-%% @spec handle_event(Event, State) -> {ok, State} |
-%% {swap_handler, Args1, State1, Mod2, Args2} |
-%% remove_handler.
-%%
-%% @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
-%%--------------------------------------------------------------------
+%%%
+%%% handle_event
+%%%
+
handle_event2(Symbol, Pid, Type, Message, State) -> % Message must be a string
{ok, MessageSafe, NL} = regexp:gsub(Message, "[\n]+", " "), % collapse whitespace to one space
@@ -140,31 +121,14 @@
end,
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, {Kind, E}]} when Kind == timeout; Kind == thrift_exception ->
+ ok;
- 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, [Ref, LastMessage, Obj, Reason]} ->
+ Format1 = "** gen_server terminating in message ~p~n** State = ~s~n** Reason = ~p~n",
+ Message = sformat(Format1, [LastMessage, oop:inspect(Obj), Reason]),
+ handle_event2(Symbol, Ref, "", Message, State);
+
{?GS_TERM_FORMAT, _Dta} ->
Message = sformat("DATA DIDN'T MATCH: ~p~n", [Data]) ++ sformat(Format, Data),
handle_event2(Symbol, Ref, "", Message, State);
@@ -213,58 +177,23 @@
{ok, State}
end.
-%%--------------------------------------------------------------------
-%% @spec handle_call(Request, State) -> {ok, Reply, State} |
-%% {swap_handler, Reply, Args1, State1,
-%% Mod2, Args2} |
-%% {remove_handler, Reply}.
-%%
-%% @doc
-%% Whenever an event manager receives a request sent using
-%% gen_event:call/3,4, this function is called for the specified event
-%% handler to handle the request.
-%% @end
-%%--------------------------------------------------------------------
+%%%
+%%% call, info, terminate, code_change
+%%%
+
handle_call(_Request, State) ->
Reply = ok,
{ok, Reply, State}.
-%%--------------------------------------------------------------------
-%% @spec handle_info(Info, State) -> {ok, State} |
-%% {swap_handler, Args1, State1, Mod2, Args2} |
-%% remove_handler.
-%%
-%% @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
-%%--------------------------------------------------------------------
handle_info(_Info, State) ->
{ok, State}.
-%%--------------------------------------------------------------------
-%% @spec terminate(Reason, State) -> void().
-%%
-%% @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
-%%--------------------------------------------------------------------
terminate(normal, _State) ->
ok;
terminate(Reason, _State) ->
format("*****************~n~n frick, error logger terminating: ~p~n~n*****************~n~n", [Reason]),
ok.
-%%--------------------------------------------------------------------
-%% @spec code_change(OldVsn, State, Extra) -> {ok, NewState}.
-%%
-%% @doc
-%% Convert process state when code is changed
-%% @end
-%%--------------------------------------------------------------------
code_change(_OldVsn, State, _Extra) ->
{ok, State}.
@@ -285,13 +214,10 @@
print_crash_report(Report) ->
case Report of
- [[_,_,{error_info, XX}|_] | _] ->
- case thrift_utils:first_item(XX) of
- tTransportException ->
- ok;
- _ ->
- io:format("~~~~ crash report: ~P~n", [XX, 3])
- end;
+ [[_,_,{error_info, {thrift_exception, _}}|_] | _] ->
+ ok;
+ [[_,_,{error_info, {timeout, _}}|_] | _] ->
+ ok;
_ ->
- io:format("~~~~ crash report (?): ~p~n", [Report])
+ io:format("~~~~ crash report: ~w~n", [Report])
end.
diff --git a/lib/erl/src/thrift_oop_server.erl b/lib/erl/src/thrift_oop_server.erl
index 052578c..d3bc720 100644
--- a/lib/erl/src/thrift_oop_server.erl
+++ b/lib/erl/src/thrift_oop_server.erl
@@ -4,77 +4,43 @@
%%% See accompanying file LICENSE or visit the Thrift site at:
%%% http://developers.facebook.com/thrift/
-%%%-------------------------------------------------------------------
-%%% @doc
-%%% @end
-%%%-------------------------------------------------------------------
-module(thrift_oop_server).
-behaviour(gen_server).
-%%--------------------------------------------------------------------
-%% Include files
-%%--------------------------------------------------------------------
+
-include("oop.hrl").
-include("thrift.hrl").
-%%--------------------------------------------------------------------
-%% External exports
-%%--------------------------------------------------------------------
+-include("transport/tTransportException.hrl").
+-include("protocol/tProtocolException.hrl").
+
-export([
start_link/0,
stop/0
]).
-%%--------------------------------------------------------------------
-%% gen_server callbacks
-%%--------------------------------------------------------------------
-export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2, code_change/3]).
-%%--------------------------------------------------------------------
-%% record definitions
-%%--------------------------------------------------------------------
-
-%%--------------------------------------------------------------------
-%% macro definitions
-%%--------------------------------------------------------------------
-define(SERVER, ?MODULE).
-%%====================================================================
-%% External functions
-%%====================================================================
-%%--------------------------------------------------------------------
-%% @doc Starts the server.
-%% @spec start_link() -> {ok, pid()} | {error, Reason}
-%% @end
-%%--------------------------------------------------------------------
+%%%
+%%% api
+%%%
+
start_link() ->
gen_server:start_link({local, ?SERVER}, ?MODULE, [], []).
-%%--------------------------------------------------------------------
-%% @doc Stops the server.
-%% @spec stop() -> ok
-%% @end
-%%--------------------------------------------------------------------
stop() ->
gen_server:cast(?SERVER, stop).
-%%====================================================================
-%% Server functions
-%%====================================================================
-
-%%--------------------------------------------------------------------
-%% Function: init/1
-%% Description: Initiates the server
-%% Returns: {ok, State} |
-%% {ok, State, Timeout} |
-%% ignore |
-%% {stop, Reason}
-%%--------------------------------------------------------------------
+%%%
+%%% init
+%%%
init({Class, Args}) ->
process_flag(trap_exit, true),
- try
+ try %% TODO use apply_if_defined
State = apply(Class, new, Args),
?INFO("thrift ~p:new(~s) = ~s", [Class, thrift_utils:unbrack(Args), oop:inspect(State)]),
{ok, State}
@@ -85,132 +51,102 @@
init(_) ->
{stop, invalid_params}.
-%%--------------------------------------------------------------------
-%% Function: handle_call/3
-%% Description: Handling call messages
-%% Returns: {reply, Reply, State} |
-%% {reply, Reply, State, Timeout} |
-%% {noreply, State} |
-%% {noreply, State, Timeout} |
-%% {stop, Reason, Reply, State} | (terminate/2 is called)
-%% {stop, Reason, State} (terminate/2 is called)
-%%--------------------------------------------------------------------
+%%%
+%%% call and cast
+%%%
-handle_call(Request, From, State) ->
- handle_either(call, Request, From, State).
+handle_call(Request, From, State) -> handle_call_cast(call, Request, From, State).
+handle_cast(stop, State) -> {stop, normal, State};
+handle_cast({Method, Args}, State) -> handle_call_cast(cast, {Method, Args}, undefined, State).
-%%--------------------------------------------------------------------
-%% Function: handle_cast/2
-%% Description: Handling cast messages
-%% Returns: {noreply, State} |
-%% {noreply, State, Timeout} |
-%% {stop, Reason, State} (terminate/2 is called)
-%%--------------------------------------------------------------------
+reply(call, Value, State) -> {reply, Value, State};
+reply(cast, _Value, State) -> {noreply, State}.
-handle_cast(stop, State) ->
- {stop, normal, State};
-
-handle_cast({Method, Args}, State) ->
- handle_either(cast, {Method, Args}, undefined, State).
-
--define(REPLY(Value, State),
- case Type of
- call -> {reply, Value, State};
- cast -> {noreply, State}
- end
-).
-
-handle_either(Type, Request, From, State) ->
- %% error_logger:info_msg("~p: ~p", [?SERVER, oop:inspect(State)]),
- %% error_logger:info_msg("handle_call(Request=~p, From=~p, State)", [Request, From]),
+handle_call_cast(Type, Request, From, State) ->
+ %% ?INFO("~p: ~p", [?SERVER, oop:inspect(State)]),
+ %% ?INFO("handle_call(Request=~p, From=~p, State)", [Request, From]),
case Request of
{get, [Field]} ->
Value = oop:get(State, Field),
- ?REPLY(Value, State);
+ reply(Type, Value, State);
{set, [Field, Value]} ->
State1 = oop:set(State, Field, Value),
- ?REPLY(Value, State1);
+ reply(Type, Value, State1);
{class, []} ->
- ?REPLY(?CLASS(State), State);
+ reply(Type, ?CLASS(State), State);
{Method, Args} ->
handle_method(Type, State, Method, Args);
_ ->
- error_logger:format("no match for Request = ~p", [Request]),
- %% {stop, server_error, State}
- {reply, server_error, State}
+ ?ERROR("thrift no match for Request = ~p", [Request]),
+ {stop, server_error, State}
+ %% {reply, server_error, State}
end.
handle_method(Type, State, Method, Args) ->
- %% is an effectful call?
Is_effectful = lists:prefix("effectful_", atom_to_list(Method)),
- Call = oop:call(State, Method, Args),
- %% TODO(cpiro): maybe add error handling here? = catch oop:call?
-
- case {Is_effectful, Call} of
+ try {Is_effectful, oop:call(State, Method, Args)} of
{true, {Retval, State1}} ->
- ?REPLY(Retval, State1);
+ reply(Type, Retval, State1);
{true, _MalformedReturn} ->
%% TODO(cpiro): bad match -- remove when we're done converting
- error_logger:format("oop:call(effectful_*,..,..) malformed return value ~p",
- [_MalformedReturn]),
- %% {stop, server_error, State}
- {noreply, State};
+ ?ERROR("oop:call(effectful_*,..,..) malformed return value ~p",
+ [_MalformedReturn]),
+ {stop, server_error, State};
+ %% {noreply, State};
{false, Retval} ->
- ?REPLY(Retval, State)
+ reply(Type, Retval, State)
+
+ catch
+ exit:{thrift_exception, E} -> handle_exception(E, nothing);
+ exit:{{thrift_exception, E}, Stack} -> handle_exception(E, Stack);
+ exit:normal -> exit(normal);
+ exit:(X = {timeout, _}) -> exit(X);
+ exit:Other ->
+ exit(Other)
end.
-%%--------------------------------------------------------------------
-%% Function: handle_info/2
-%% Description: Handling all non call/cast messages
-%% Returns: {noreply, State} |
-%% {noreply, State, Timeout} |
-%% {stop, Reason, State} (terminate/2 is called)
-%%--------------------------------------------------------------------
-handle_info({'EXIT', Pid, Except} = All, State) ->
- Result = try
- oop:call(State, catches, [Pid, Except])
- catch
- exit:{missing_method, _} ->
- unhandled
- end,
+handle_exception(E, Stack) ->
+ %% ?ERROR("texception ~p", [E]),
+ case {oop:is_a(E, tException), Stack} of
+ {true, nothing} -> % good
+ exit({thrift_exception, E});
+ {true, _} -> % good
+ E1 = tException:add_backtrace_element(E, Stack),
+ exit({thrift_exception, E1});
+ false -> % shit
+ ?ERROR("exception wasn't really a tException ~p", [E]),
+ exit(bum)
+ end.
- case Result of
- unhandled ->
+%%%
+%%% info, terminate, and code_change
+%%%
+
+handle_info({'EXIT', Pid, Except} = All, State) ->
+ case Except of
+ normal ->
+ {noreply, State};
+ {normal, _} ->
+ {noreply, State};
+ _unhandled ->
error_logger:format("unhandled exit ~p", [All]),
- {stop, All, State};
- _WasHandled ->
- {noreply, State}
+ {stop, All, State}
end;
handle_info(Info, State) ->
- error_logger:info_msg("~p", [Info]),
+ ?INFO("~p", [Info]),
{noreply, State}.
-%%--------------------------------------------------------------------
-%% Function: terminate/2
-%% Description: Shutdown the server
-%% Returns: any (ignored by gen_server)
-%%--------------------------------------------------------------------
terminate(Reason, State) ->
- %%error_logger:format("~p terminated!: ~p", [self(), Reason]),
ok.
-%%--------------------------------------------------------------------
-%% Func: code_change/3
-%% Purpose: Convert process state when code is changed
-%% Returns: {ok, NewState}
- %%--------------------------------------------------------------------
code_change(OldVsn, State, Extra) ->
{ok, State}.
-
-%%====================================================================
-%%% Internal functions
-%%====================================================================
diff --git a/lib/erl/src/thrift_sup.erl b/lib/erl/src/thrift_sup.erl
index 8f6eaf7..ab3f80e 100644
--- a/lib/erl/src/thrift_sup.erl
+++ b/lib/erl/src/thrift_sup.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/
@@ -29,7 +29,7 @@
Args = [SF, Port, Handler, Processor, ST, TF, PF],
ThriftServer = {thrift_server, {?MODULE, thrift_start_link, Args},
- permanent, 2000, worker, ThriftModules},
+ permanent, 2000, worker, ThriftModules},
{ok, {{one_for_one, 10, 1}, [ThriftServer]}}.
diff --git a/lib/erl/src/thrift_utils.erl b/lib/erl/src/thrift_utils.erl
index 8305224..1cbacc0 100644
--- a/lib/erl/src/thrift_utils.erl
+++ b/lib/erl/src/thrift_utils.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/
diff --git a/lib/erl/src/transport/tErlAcceptor.erl b/lib/erl/src/transport/tErlAcceptor.erl
index 8398924..bcfcb9a 100644
--- a/lib/erl/src/transport/tErlAcceptor.erl
+++ b/lib/erl/src/transport/tErlAcceptor.erl
@@ -10,6 +10,7 @@
-include("thrift.hrl").
-include("tApplicationException.hrl").
-include("transport/tTransportException.hrl").
+-include("protocol/tProtocolException.hrl").
-include("transport/tServerSocket.hrl").
-include("transport/tErlAcceptor.hrl").
@@ -69,14 +70,13 @@
case catch gen_tcp:accept(ListenSocket) of
{ok, Socket} ->
- ?C0(ServerPid, effectful_new_acceptor), %% cast to create new acceptor
+ ?C0(ServerPid, effectful_new_acceptor), % cast to create new acceptor
AddrString = render_addr(Socket),
?INFO("thrift connection accepted from ~s", [AddrString]),
- %% start_new(tSocket, [])
Client = oop:start_new(tSocket, []),
- ?R1(Client, effectful_setHandle, Socket), %% TODO(cpiro): should we just let this be a param to the constructor?
+ ?R1(Client, effectful_setHandle, Socket),
%% cpiro: OPAQUE!! Trans = Client
TF = oop:get(This, transportFactory),
@@ -87,47 +87,66 @@
Prot = ?F1(PF, getProtocol, Trans),
%% start_new(, ...)
- Processor = oop:start_new(tErlProcessor, [GP, Handler]), %% TODO
+ Processor = oop:start_new(tErlProcessor, [GP, Handler]),
- case receive_loop(This, Processor, Prot, Prot) of
- conn_timeout ->
+ try
+ receive_loop(This, Processor, Prot, Prot)
+ catch
+ exit:{timeout, _} ->
?INFO("thrift connection timed out from ~s", [AddrString]);
- conn_closed ->
- ?INFO("thrift connection closed from ~s", [AddrString]);
- {Class, Else} ->
- ?ERROR("unhandled ~p in tErlAcceptor: ~p", [Class, Else])
+
+ %% cpiro: i think the extra entry on the stack is always from receive_loop
+ %% the below case shouldn't happen then? if we move this catch inside
+ %% we'll probably need this case and not the next one
+
+ %% exit:{thrift_exception, E} ->
+ %% handle_exception(E, AddrString, no2);
+
+ exit:{{thrift_exception, E}, Stack1} ->
+ handle_exception(E, AddrString, Stack1);
+
+ Class:Else ->
+ ?ERROR("some other error ~p in tErlAcceptor: ~p", [Class, Else])
end,
exit(normal);
Else ->
R = thrift_utils:sformat("accept() failed: ~p", [Else]),
- exit(tTransportException:new(R))
+ tException:throw(tTransportException, [R])
end.
+
+handle_exception(E, AddrString, Stack1) ->
+ case tException:read(E) of
+ none -> % not a tException
+ ?ERROR("not really a tException: ~p", [exit, E]);
+
+ {tProtocolException, ?tProtocolException_BAD_VERSION, _} ->
+ ?INFO("thrift missing version from ~s", [AddrString]);
+
+ {tTransportException, ?tTransportException_NOT_OPEN, _} ->
+ ?INFO("thrift connection closed from ~s", [AddrString]);
+
+ _ ->
+ Where = "thrift tErlAcceptor caught a tException",
+ ?ERROR("~s", [tException:inspect_with_backtrace(E, Where, Stack1)])
+ end.
+
+%% always calls itself ... only way to escape is through an exit
receive_loop(This, Processor, Iprot, Oprot) ->
- try ?R2(Processor, process, Iprot, Oprot) of
- {error, TAE} when is_record(TAE, tApplicationException),
- TAE#tApplicationException.type == ?tApplicationException_HANDLER_ERROR ->
- ?ERROR("thrift handler returned an error: ~p", [oop:get(TAE, message)]),
+ case ?R2(Processor, process, Iprot, Oprot) of
+ {error, Reason} ->
+ case tException:read(Reason) of
+ none ->
+ ?ERROR("thrift handler returned something weird: {error, ~p}", [Reason]);
+ _ ->
+ Where = "thrift processor/handler caught a tException",
+ ?ERROR("~s", [tException:inspect_with_backtrace(Reason, Where)])
+ end,
receive_loop(This, Processor, Iprot, Oprot);
Value ->
?INFO("thrift request: ~p", [Value]),
receive_loop(This, Processor, Iprot, Oprot)
- catch
- exit:{timeout, _} ->
- conn_timeout;
-
- %% the following clause must be last
- %% cpiro: would be best to implement an is_a/2 guard BIF
- %% cpiro: breaks if it's a subclass of tTransportException
- %% since unnest_record knows nothing about oop
- Class:Else ->
- case thrift_utils:unnest_record(Else, tTransportException) of
- {ok, TTE} when TTE#tTransportException.type == ?tTransportException_NOT_OPEN ->
- conn_closed;
- _ ->
- {Class, Else}
- end
end.
%% helper functions
diff --git a/lib/erl/src/transport/tSocket.erl b/lib/erl/src/transport/tSocket.erl
index f002fd8..5aead1e 100644
--- a/lib/erl/src/transport/tSocket.erl
+++ b/lib/erl/src/transport/tSocket.erl
@@ -75,10 +75,8 @@
case gen_tcp:connect(Host, Port, Options) of
{error, _} ->
- exit(tTransportException:new(
- ?tTransportException_NOT_OPEN,
- "Could not connect to " ++ Host ++ ":" ++ Port)
- );
+ tException:throw(tTransportException,
+ [?tTransportException_NOT_OPEN, "Could not connect to " ++ Host ++ ":" ++ Port]);
{ok, Socket} ->
effectful_setHandle(This, Socket)
end.
@@ -97,7 +95,7 @@
case Val of
{error, _} ->
- throw(tTransportException:new(?tTransportException_NOT_OPEN, "in write"));
+ tException:throw(tTransportException, [?tTransportException_NOT_OPEN, "in write"]);
ok ->
{ok, This}
end.
@@ -108,13 +106,13 @@
{ok, []} ->
Host = oop:get(This, host),
Port = oop:get(This, port),
- throw(tTransportException:new(?tTransportException_UNKNOWN, "TSocket: Could not read " ++ Sz ++ "bytes from " ++ Host ++ ":" ++ Port));
+ tException:throw(tTransportException, [?tTransportException_UNKNOWN, "TSocket: Could not read " ++ Sz ++ "bytes from " ++ Host ++ ":" ++ Port]);
{ok, Data} ->
%% DEBUG
?INFO("tSocket: read ~p", [Data]),
Data;
{error, Error} ->
- exit(tTransportException:new(?tTransportException_NOT_OPEN, "in tSocket:read/2: gen_tcp:recv"))
+ tException:throw(tTransportException, [?tTransportException_NOT_OPEN, "in tSocket:read/2: gen_tcp:recv"])
end.
effectful_close(This) ->
diff --git a/lib/erl/src/transport/tTransportException.erl b/lib/erl/src/transport/tTransportException.erl
index b7dc43d..f81ae9b 100644
--- a/lib/erl/src/transport/tTransportException.erl
+++ b/lib/erl/src/transport/tTransportException.erl
@@ -22,8 +22,7 @@
%%% 'super' is required unless ?MODULE is a base class
%%%
-?DEFINE_ATTR(super);
-?DEFINE_ATTR(type).
+?DEFINE_ATTR(super).
%%%
%%% behavior callbacks
@@ -38,15 +37,15 @@
%%% inspect(This) -> string()
inspect(This) ->
- ?FORMAT_ATTR(type).
+ "".
%%%
%%% class methods
%%%
new(Type, Message) ->
- Super = (super()):new(Message),
- #?MODULE{super=Super, type=Type}.
+ Super = (super()):new(Type, Message),
+ #?MODULE{super=Super}.
new() ->
new(?tTransportException_UNKNOWN, undefined).