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