[erlang-questions] erlang:raise/3 considered harmful?

[Ryan Stewart]

+1 for "exceptions suck and lead to imperative programming patterns". That
said, I'm still trying to work out the Right Way to do error handling in
Erlang. On a quick scan, I like Garrett's suggestions, although I would
clarify it by saying that like dict:find() vs dict:fetch(), it might be
reasonable for your API to support both a "crash on failure" and a "return
tagged tuple on failure" option. That gives callers more freedom in how
they use your code. To me, this feels like the age-old "checked vs
unchecked" exception problem, and that's all about how you force callers to
interact with your code.

On Sat, May 14, 2016 at 11:20 AM Garrett Smith <g@REDACTED> wrote:

> On Sat, May 14, 2016 at 4:48 AM, Per Hedeland <per@REDACTED> wrote:
> > Hi,
> >
> > I happened to want something like erlang:raise/3 for a library function
> > - I didn't think it existed:-), but found that it does, although it
> > comes with a semi-dire warning. The loop/3 function below does exactly
> > what I want, specifically: if the fun provided by the caller of
> > interate/3 crashes, the caller gets the same exception as if there had
> > been no try ... catch, can handle it with hir own try/catch or not, but
> > in any case receives full information about *what* went wrong in hir fun
> > - all while the "protocol" spoken over the socket remains in sync.
> >
> > So, to take heed of the warning - is there a way to write an equivalent
> > function *without* using raise/3 - or should it just be considered a
> > case of "you really know what you are doing"? I believe I am of
> > course:-), but I also think that the code below is a perfectly
> > reasonable implementation of a side-effecty library function, and has no
> > more to do with debugging or knowing what you're doing than any other
> > code...
>
> I think your case justifies the use of this scary function :)
>
> That said, I don't think you actually want to use it. Your API need
> improving I think.
>
> The fact you're returning a tagged tuple {ok, State} puts this into
> the "tagged tuple" function category. This in contrast to the "value
> vs exception" category. See dict:find vs dict:fetch for a contrast of
> the two patterns. Stick with one or the other - don't mingle them.
>
> So I'd expect something like this:
>
> try
>   Fun(Data, State)
> catch
>   Class:Reason ->
>     Err = {Class, Reason, erlang:get_stacktrace()}
>     {error, {callback_error, Err}}
> end
>
> But onto other issues...
>
> It's not clear what "Error" is from looking at the code - and I'd
> rather not (as a reader) have to poke around type specs. If you expect
> the form {error, Error} there, spell that out. E.g.
>
>             case Res of
>                 ...
>                 {error, Error} ->
>                     gen_tcp:send(Sock, "done"),
>                     {error, Error}
>
> If "Error" can be anything, you're on shaky ground API wise. You're
> asking the caller of your function to handle {ok, Good}, arbitrary
> exceptions, and arbitrary return values. I would be very cranky with
> this interface.
>
> Finally, the imperative style coding here really wants some functions,
> in particular the calling and handling of the callback (but also
> applies to the calling and handling of the socket read). I'd do
> something like this:
>
> handle_cb(apply_cb(Fun, Data, State), Socket, Fun)
>
> apply_cb can then tuck away your logic for how to call this function.
> I'd typically use "catch" (I don't care for try...catch), but this
> would allow "throw" errors to pass through as results. You may or may
> not want that - I would not want that certainly without a strong
> defense. So this:
>
> apply_cb(Fun, Data, State) ->
>   try
>     Fun(Data, State)
>   catch
>     Class:Reason ->
>       {error, {cb_error, {Class, Reason, erlang:get_stacktract()}}}
>   end.
>
> Your handler then replaces your case expression:
>
> handle_cb({continue, State}, Sock, Fun) ->
>   gen_tcp:send(Sock, "continue"),
>   loop(Sock, Fun, State);
> handle_cb({done, State}, Sock, Fun) ->
>   gen_tcp:send(Sock, "done"),
>   {ok, State};
> handle_cb({error, Err}, Sock, _Fun) ->
>   gen_tcp:send(Sock, "done"),
>   {error, Error};
> handle_db(Other, Sock, _Fun) ->
>   gen_tcp:send(Sock, "done"),
>   error({unexpected_cb_result, Other}).
>
> You get a lot of goodness here:
>
> - Clearly separated concerns, each with its own name+args (i.e. function)
> - Consistent use of tagged tuple pattern ({ok, Good}, {error, Bad})
> - Explicit error information with enough detail to diagnose errors
> - Intolerance and refusal to handle a misuse of this API (the error
> for unexpected cb result)
>
> And then just to see it in print:
>
> loop(Sock, Fun, State) ->
>   handle_data(recv_data(Sock), Sock, Fun, State).
>
> You get the idea.
>
> And I fully understand, this was not your original question :)
> _______________________________________________
> erlang-questions mailing list
> erlang-questions@REDACTED
> http://erlang.org/mailman/listinfo/erlang-questions
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-questions/attachments/20160514/cd74064e/attachment.htm>