[erlang-questions] Error handling with good looking code?

[Ciprian Dorin Craciun]

On Thu, Nov 3, 2011 at 15:08, Attila Rajmund Nohl
<attila.r.nohl@REDACTED> wrote:
> Hello!
>
> I have this code (cut out some non-interesting pieces):
>
> upload_trace_record_file2(Host, Usr, Pwd, RemDir, NetTraceRef) ->
>    case sftp_client:open(Host, ?SFTP_PORT, Usr, Pwd) of
>        {ok, Pid} ->
> ....
>            case sftp_client:send(Pid, TRFile, RemTRFile) of
>                ok ->
>                    ok;
>                {error, Reason} ->
>                    {error, sftp_client:format_error(Reason)}
>            end,
>            sftp_client:close(Pid),
>            file:delete(TRFile);
>        {error, Reason} ->
>            {error, sftp_client:format_error(Reason)}
>    end.
>
> [...]
>
> There are at least two obvious bugs in the error handling (the error
> from sftp_client:send is not used; the update_trace_record_file throws
> an exception instead of returning an error), but it made think about
> how to handle the errors properly in this code. The "let it crash"
> philosophy doesn't really work here, because we need to produce a
> meaningful error message to the user and the stack trace is not a
> meaningful message to them. I was thinking something like this:
>
> upload_trace_record_file2(Host, Usr, Pwd, RemDir, NetTraceRef) ->
>    try
>        {sftp_client, {ok, Pid}} = {sftp_client,
> sftp_client:open(Host, ?SFTP_PORT, Usr, Pwd)},
> ....
>        {sftp_client, ok} = {sftp_client, sftp_client:send(Pid,
> TRFile, RemTRFile)},
>        sftp_client:close(Pid),
>        file:delete(TRFile)
>   catch
>        error:{badmatch, {sftp_client, Reason}} ->
>            {error, sftp_client:format_error(Reason)};
>        error:{badmatch, {exml, _}} ->
>                  {error, bad_xml_file}
>    end.
>
> [...]
>
> but it looks somewhat ugly. Three "assignments" and function calls are
> hidden away in tuples, so it's a little hard to read, even though the
> rest of the code is simpler. Do you have any other ideas?


    This is exactly with what I was struggling a few weeks ago... And
I've found something similar with your `try / catch` solution, but I
think it's simpler:
    * I've defined some functions `enforce_ok_N` which take one input
in the form of `{ok, X1, X2, ..., XN}` (where N is the length of the
tuple I expect) or `{error, _Reason}` and in case of `ok` it just
removes the `ok` from the tuple (`enforce_ok_1` particularly just
returns the `X1` value), but in case of error it just throws it;
    * I use it in a non-nested (almost imperative style) but wrapped
in a single try / catch;

~~~~
handle_info (
                        {mosaic_component_backend_internals,
push_packet, Packet},
                        State = #state{harness = Harness})
                when is_pid (Harness) ->
        try
                EncodedPacket = enforce_ok_1
(mosaic_component_coders:encode_packet_fully (Packet)),
                ok = enforce_ok (mosaic_harness_backend:push_packet
(Harness, EncodedPacket)),
                {noreply, State}
        catch throw : Error = {error, Reason} ->
                ok = mosaic_transcript:trace_error ("failed encoding
packet; terminating!", [{packet, Packet}, {reason, Reason}]),
                {stop, Error, State}
        end;
~~~~

    Code is at (with no dependencies):
        https://bitbucket.org/cipriancraciun/mosaic-node/src/673b85c651ab/applications/mosaic-tools/sources/mosaic_enforcements.erl

    The advantages of my solution is twofold:
    1) it is usable with almost all existing Erlang functions (as most
of them just respect the `{ok, ...}` or `{error, Reason}` convention);
    2) the functions written by using the `enforce_ok_X` (assuming you
use the outermost `try`, never throw, and behave just like other
normal functions;

    Hope this helps,
    Ciprian.

    P.S.: If I write a deeply recursive function that returns `{ok,
...} | {error, ...}` I do something slightly different: I create a
wrapper function that just calls an `do_something_ok_1`, which throws
`{error, Reason}`, or always returns a term corresponding to the ok
value:
        https://bitbucket.org/cipriancraciun/mosaic-node/src/673b85c651ab/applications/mosaic-tools/sources/mosaic_generic_coders.erl#cl-139

~~~~
encode_text_term (Term) ->
        try
                {ok, erlang:iolist_to_binary (encode_text_term_ok_1 (Term))}
        catch throw : Error = {error, _Reason} -> Error end.

encode_text_term_ok_1 (Atom)
                when is_atom (Atom) ->
        [$', erlang:atom_to_binary (Atom, utf8), $'];

...
encode_text_term_ok_1 (Term) ->
        throw ({error, {invalid_text, Term}}).
~~~~