Re-raise original error in mimemail:decode_header/2

[maltoe]

Hello 👋

I had an interesting debugging session today due an odd inexplicable error within mimemail:decode/3:

** (UndefinedFunctionError) function :mimemail.decode_header/2 is undefined or private
    (gen_smtp 1.1.1) testproject/deps/gen_smtp/src/mimemail.erl:198: :mimemail.decode_header/2
    (gen_smtp 1.1.1) testproject/deps/gen_smtp/src/mimemail.erl:183: :mimemail.decode_headers/3
    (gen_smtp 1.1.1) testproject/deps/gen_smtp/src/mimemail.erl:130: :mimemail.decode/3

After this change

** (UndefinedFunctionError) function :iconv.convert/3 is undefined (module :iconv is not available)
    :iconv.convert("UTF-8", "utf-8//IGNORE", "Confirm your email address")
    (gen_smtp 1.1.1) testproject/deps/gen_smtp/src/mimemail.erl:282: :mimemail.convert/3
    (gen_smtp 1.1.1) testproject/deps/gen_smtp/src/mimemail.erl:249: :mimemail.decode_header_tokens_strict/2
    (gen_smtp 1.1.1) testproject/deps/gen_smtp/src/mimemail.erl:188: :mimemail.decode_header/2
    (gen_smtp 1.1.1) testproject/deps/gen_smtp/src/mimemail.erl:183: :mimemail.decode_headers/3
    (gen_smtp 1.1.1) testproject/deps/gen_smtp/src/mimemail.erl:130: :mimemail.decode/3

Or is there any reason not to do this? As far as I can tell Type:Reason:Stacktrace is support since OTP 21, so in earlier versions this catch clause wouldn't match, but I guess the error would still be better than the stacktrace-less rethrow? 🤔 Maybe I'm overlooking something.

Thanks for making this library though!

malte

[mworrell]

@seriyps @arjan To use this we need to drop support for OTP20, as OTP24 is out we might consider dropping 20 support?

[mworrell]

@maltoe thanks, please note that edoc uses html-ish notation and not markdown, I think that is the reason why the test on the docs is failing.

[maltoe]

@mworrell Sorry, you're right it doesn't even compile on OTP20, should have checked that before. This would make it compatible to OTP<=20, but not sure if you would want that?

diff --git a/src/mimemail.erl b/src/mimemail.erl
index 0c5559d..aae3536 100644
--- a/src/mimemail.erl
+++ b/src/mimemail.erl
@@ -80,6 +80,13 @@
                {default_mime_version, ?DEFAULT_MIME_VERSION} % default mime version
        ]).

+% Both the :Stacktrace syntax for catch and the ?OTP_RELEASE macro were introduced in OTP21.
+-ifdef(OTP_RELEASE).
+-define(EXC_WITH_STACK(Type, Reason, Stacktrace), Type:Reason:Stacktrace ->).
+-else.
+-define(EXC_WITH_STACK(Type, Reason, Stacktrace), Type:Reason -> Stacktrace = erlang:get_stacktrace(),).
+-endif.
+
 -type mime_type() :: binary().                  % `<<"text">>'
 -type mime_subtype() :: binary().               % `<<"plain">>'
 -type headers() :: [{binary(), binary()}].      % `[{<<"Content-Type">>, <<"text/plain">>}]'
@@ -190,12 +197,12 @@ decode_header(Value, Charset) ->
        RTokens = tokenize_header(Value, []),
        Tokens = lists:reverse(RTokens),
        Decoded = try decode_header_tokens_strict(Tokens, Charset)
-               catch Type:Reason:Stacktrace ->
+               catch ?EXC_WITH_STACK(T, R, S)
                        case decode_header_tokens_permissive(Tokens, Charset, []) of
                                {ok, Dec} -> Dec;
                                error ->
                                        % re-throw original error
-                                       erlang:raise(Type, Reason, Stacktrace)
+                                       erlang:raise(T, R, S)
                        end
                end,
        iolist_to_binary(Decoded).

Unfortunately on newer OTP releases this produces a deprecation warning:

===> Compiling gen_smtp
../../../../usr/local/lib/erlang/lib/parsetools-2.1.6/include/yeccpre.hrl:60:26: Warning: erlang:get_stacktrace/0 is removed; use the new try/catch syntax for retrieving the stack backtrace

[maltoe]

@mworrell Any news? Would you like me to implement the backwards compatible version as outlined above?

[mworrell]

@seriyps what do you think? I think that for the 1.0 branch it should be ok to only support 22, 23, and 24. As does rebar3.

That would make hacks for the stack trace unneeded.

/cc @arjan

[mworrell]

We can merge this after we changed #273 to drop support for OTP20.

[mworrell]

@maltoe Thanks!