2

I'm using a oneway modifier in one of my Thrift function definitions:

...
oneway void secret_function(1: string x, 2: string y),
...

When generating the respective Erlang code via Thrift, this is translated into:

...
function_info('secret_function', reply_type) ->
  oneway_void;
function_info('secret_function', exceptions) ->
  {struct, []};
...

Please note the oneway_void atom there.

When the secret_function function is executed, I get the following error:

=ERROR REPORT==== 2-Sep-2010::18:17:08 ===
oneway void secret_function threw error which must be ignored: {error,
                                                             function_clause,
                                                             [{thrift_protocol,
                                                               term_to_typeid,
                                                               [oneway_void]},
                                                              {thrift_protocol,
                                                               struct_write_loop,
                                                               3},
                                                              {thrift_protocol,
                                                               write,2},
                                                              {thrift_processor,
                                                               send_reply,
                                                               4},
                                                              {thrift_processor,
                                                               handle_function,
                                                               2},
                                                              {thrift_processor,
                                                               loop,1}]}

Independently from the possible bugs contained in the user code, here the thrift_protocol:term_to_typeid/1 function is being called with the oneway_void atom as an argument, which causes a function clause. In fact, reading from the code (thrift_protocol.erl):

...
term_to_typeid(void) -> ?tType_VOID;
term_to_typeid(bool) -> ?tType_BOOL;
term_to_typeid(byte) -> ?tType_BYTE;
term_to_typeid(double) -> ?tType_DOUBLE;
term_to_typeid(i16) -> ?tType_I16;
term_to_typeid(i32) -> ?tType_I32;
term_to_typeid(i64) -> ?tType_I64;
term_to_typeid(string) -> ?tType_STRING;
term_to_typeid({struct, _}) -> ?tType_STRUCT;
term_to_typeid({map, _, _}) -> ?tType_MAP;
term_to_typeid({set, _}) -> ?tType_SET;
term_to_typeid({list, _}) -> ?tType_LIST.
...

A bug? Any other explanation? Why is oneway_void being passed to that function?

Roberto Aloi
  • 30,570
  • 21
  • 75
  • 112
  • As far as I know Erlang thrift bindings are quite mediocre, so it's quite possible it's a bug – gleber Sep 02 '10 at 18:27
  • What does `secret_function/2` look like? How does it call `term_to_typeid/1`? I have no knowledge of thrift but looking at the functions should tell you. – rvirding Sep 02 '10 at 21:33
  • @rvirding: I've added the full stack trace. There's nothing there directly related to my own functions. My feeling is that something is wrong in the secret_function/2, and that the real error is "eaten" by Thrift, which is printing just that section of the stack trace, making hard to identify the reason of the error itself. Trying to do some tracing now... – Roberto Aloi Sep 06 '10 at 11:45

1 Answers1

0

I think I know what's going on behind the scenes.

My Erlang code (the secret_function/2) was returning {ok, pid()} rather than simply ok. Even if this is conceptually wrong since I declared the function oneway_void, it took me a while to identify the cause of the problem. Maybe we could adjust the handle_succes function in Thrift so that it behaves the same way the handle_function_catch already does. This is how the handle_function_catch looks like at the moment:

...
case {ErrType, ErrData} of
    _ when IsOneway ->
        Stack = erlang:get_stacktrace(),
        error_logger:warning_msg(
          "oneway void ~p threw error which must be ignored: ~p",
          [Function, {ErrType, ErrData, Stack}]),
        {State, ok};
...

Even if the function is declared as oneway_void, when an exception is raised, the problem is reported. A potential new handle_success function, following the same reasoning, could then look like the following:

handle_success(State = #thrift_processor{service = Service},
               Function,
               Result) ->
    ReplyType  = Service:function_info(Function, reply_type),
    StructName = atom_to_list(Function) ++ "_result",

    case Result of
        {reply, ReplyData} when ReplyType =:= oneway_void ->
            Stack = erlang:get_stacktrace(),
            error_logger:warning_msg(
              "oneway void ~p sent reply which must be ignored: ~p",
              [Function, {ReplyData, Stack}]),
            {State, ok};
                {reply, ReplyData} ->
            Reply = {{struct, [{0, ReplyType}]}, {StructName, ReplyData}},
            send_reply(State, Function, ?tMessageType_REPLY, Reply);

        ok when ReplyType == {struct, []} ->
            send_reply(State, Function, ?tMessageType_REPLY, {ReplyType, {StructName}});

        ok when ReplyType == oneway_void ->
            %% no reply for oneway void
            {State, ok}
    end.

Here I'm just checking if the function is defined as oneway_void and if this is true and I still receive a return value different from the atom ok, I report the accident, still ignoring the return value.

This is what a developer would see with the updated handle_success function:

=ERROR REPORT==== 7-Sep-2010::11:06:43 ===
oneway void secret_function sent reply which must be ignored: {{ok,
                                                                  <0.262.0>},
                                                                 []}

And that could save your life at least once (cit.).

Roberto Aloi
  • 30,570
  • 21
  • 75
  • 112