[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/compiler/cpp/src/generate/t_erl_generator.cc b/compiler/cpp/src/generate/t_erl_generator.cc
index d152c18..de807a7 100644
--- a/compiler/cpp/src/generate/t_erl_generator.cc
+++ b/compiler/cpp/src/generate/t_erl_generator.cc
@@ -717,7 +717,6 @@
                  << "=" << capitalize((*fld_iter)->get_name());
     }
     f_service_ << "}," << endl;
-
     indent(f_service_) << (*f_iter)->get_name() << "_args_write(Args, Oprot)," << endl;
 
     // Write to the stream
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).