File 2431-encode-decode-Include-the-stack-trace-in-e... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 2431-encode-decode-Include-the-stack-trace-in-error-retur.patch of Package erlang (Revision d448831e03f614ee0c49b24ade3a1e91)

Currently displaying revision d448831e03f614ee0c49b24ade3a1e91 , Show latest

From 3f7d4324c7b99f93a6ad36015071276692534fa1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Wed, 15 Feb 2017 14:23:59 +0100
Subject: [PATCH 4/4] encode/decode: Include the stack trace in error returns

The generated encode/2 and decode/2 functions can return
cryptic error messages. Consider this ASN.1 spec:

T DEFINITIONS AUTOMATIC TAGS ::=
BEGIN
  S ::= SEQUENCE {
    b BOOLEAN,
    i INTEGER (1..100),
    j INTEGER (0..7),
    s OCTET STRING
  }
END

In OTP 19, the error terms will look like this:

Eshell V8.2  (abort with ^G)
1> asn1ct:compile('T', [ber]).
ok
2> rr('T').
['S']
3> 'T':encode('S', #'S'{}).
{error,{asn1,{encode_boolean,undefined}}}
4> 'T':encode('S', #'S'{b=false}).
{error,{asn1,{encode_integer,undefined}}}
5> 'T':encode('S', #'S'{b=false,i=7,j=0}).
{error,{asn1,function_clause}}

Some error terms are clearer than other. In the first error
term, it is clear that the error refers to the 'b' field,
since there is only one BOOLEAN in 'S'. The second error
term could refer to either 'i' or 'j'. The last error term...
well... in this case we can infer that it must refer to 's'.

The easiest way to provide more information is to include
the stack trace with line numbers in the error term:

3> 'T':encode('S', #'S'{b=false}).
{error,{asn1,{{encode_integer,undefined},
              [{'T',encode_integer,2,[{file,"T.erl"},{line,240}]},
               {'T',enc_S,2,[{file,"T.erl"},{line,102}]},
               {'T',encode,2,[{file,"T.erl"},{line,36}]},
               {erl_eval,do_apply,6,[{file,"erl_eval.erl"},{line,674}]},
               {shell,exprs,7,[{file,"shell.erl"},{line,686}]},
               {shell,eval_exprs,7,[{file,"shell.erl"},{line,641}]},
               {shell,eval_loop,3,[{file,"shell.erl"},{line,626}]}]}}}

By looking at the generated Erlang code, we can see that encoding
failed for 'i'.

This is an compatible change. All that the documentation says is
that the format of the error tuple is:

{error,{asn1,Description}}

With this change, Description is always a tuple:

{ErrorDescription,StackTrace}

Alternatives considered: Providing more information in the error
term itself and make sure there can be no 'function_clause', 'badarg',
or 'badmatch' exceptions. That would be possible, but it would
require a lot of work and it would increase the size of the generated
code and make it slower. Therefore, this solution was rejected.
---
 lib/asn1/doc/src/asn1_getting_started.xml |  4 ++++
 lib/asn1/src/asn1ct_gen.erl               |  7 +++---
 lib/asn1/test/ber_decode_error.erl        | 39 +++++++++++++------------------
 lib/asn1/test/testChoPrim.erl             |  8 +++----
 lib/asn1/test/testInfObjectClass.erl      | 22 ++++++++++++-----
 lib/asn1/test/testPrim.erl                |  3 ++-
 6 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/lib/asn1/doc/src/asn1_getting_started.xml b/lib/asn1/doc/src/asn1_getting_started.xml
index d2b73d63c..c036d289f 100644
--- a/lib/asn1/doc/src/asn1_getting_started.xml
+++ b/lib/asn1/doc/src/asn1_getting_started.xml
@@ -266,6 +266,10 @@ asn1ct:compile("H323-MESSAGES.asn1",[per]).      </pre>
         <c>{error, {asn1, Description}}</c> where
         <c>Description</c> is
         an Erlang term describing the error.</p>
+	<p>Currently, <c>Description</c> looks like this:
+	<c>{ErrorDescription, StackTrace}</c>. Applications should
+	not depend on the exact contents of <c>Description</c> as it
+	could change in the future.</p>
     </section>
   </section>
 
diff --git a/lib/asn1/src/asn1ct_gen.erl b/lib/asn1/src/asn1ct_gen.erl
index 9943bd056..9f628c7b0 100644
--- a/lib/asn1/src/asn1ct_gen.erl
+++ b/lib/asn1/src/asn1ct_gen.erl
@@ -802,11 +802,12 @@ result_line_1(Items) ->
 try_catch() ->
     ["  catch",nl,
      "    Class:Exception when Class =:= error; Class =:= exit ->",nl,
+     "      Stk = erlang:get_stacktrace(),",nl,
      "      case Exception of",nl,
-     "        {error,Reason}=Error ->",nl,
-     "          Error;",nl,
+     "        {error,{asn1,Reason}} ->",nl,
+     "          {error,{asn1,{Reason,Stk}}};",nl,
      "        Reason ->",nl,
-     "         {error,{asn1,Reason}}",nl,
+     "         {error,{asn1,{Reason,Stk}}}",nl,
      "      end",nl,
      "end."].
 
diff --git a/lib/asn1/test/ber_decode_error.erl b/lib/asn1/test/ber_decode_error.erl
index c0840e02d..c45d130ff 100644
--- a/lib/asn1/test/ber_decode_error.erl
+++ b/lib/asn1/test/ber_decode_error.erl
@@ -26,48 +26,41 @@ run([]) ->
     {ok,B}  = 'Constructed':encode('S3', {'S3',17}),
     [T,L|V] = binary_to_list(B),
     Bytes = list_to_binary([T,L+3|V] ++ [2,1,3]),
-    case 'Constructed':decode('S3', Bytes) of
-	{error,{asn1,{unexpected,_}}} -> ok
-    end,
+    {unexpected,_} = dec_error('S3', Bytes),
 
     %% Unexpected bytes must be accepted if there is an extensionmark
     {ok,{'S3ext',17}} = 'Constructed':decode('S3ext', Bytes),
 
     %% Truncated tag.
-    {error,{asn1,{invalid_tag,_}}} =
-	(catch 'Constructed':decode('I', <<31,255,255>>)),
+    {invalid_tag,_} = dec_error('I', <<31,255,255>>),
 
     %% Overlong tag.
-    {error,{asn1,{invalid_tag,_}}} =
-	(catch 'Constructed':decode('I', <<31,255,255,255,127>>)),
+    {invalid_tag,_} = dec_error('I', <<31,255,255,255,127>>),
 
     %% Invalid length.
-    {error,{asn1,{invalid_length,_}}} =
-	(catch 'Constructed':decode('I', <<8,255>>)),
+    {invalid_length,_} = dec_error('I', <<8,255>>),
 
     %% Other errors.
-    {error,{asn1,{invalid_value,_}}} =
-	(catch 'Constructed':decode('I', <<>>)),
+    {invalid_value,_} = dec_error('I', <<>>),
 
-    {error,{asn1,{invalid_value,_}}} =
-	(catch 'Constructed':decode('I', <<8,7>>)),
+    {invalid_value,_} = dec_error('I', <<8,7>>),
 
     %% Short indefinite length. Make sure that the decoder doesn't look
     %% beyond the end of binary when looking for a 0,0 terminator.
-    {error,{asn1,{invalid_length,_}}} =
-	(catch 'Constructed':decode('S', sub(<<8,16#80,0,0>>, 3))),
-    {error,{asn1,{invalid_length,_}}} =
-	(catch 'Constructed':decode('S', sub(<<8,16#80,0,0>>, 2))),
-    {error,{asn1,{invalid_length,_}}} =
-	(catch 'Constructed':decode('S', sub(<<40,16#80,1,1,255,0,0>>, 6))),
-    {error,{asn1,{invalid_length,_}}} =
-	(catch 'Constructed':decode('S', sub(<<40,16#80,1,1,255,0,0>>, 5))),
+    {invalid_length,_} = dec_error('S', sub(<<8,16#80,0,0>>, 3)),
+    {invalid_length,_} = dec_error('S', sub(<<8,16#80,0,0>>, 2)),
+    {invalid_length,_} = dec_error('S', sub(<<40,16#80,1,1,255,0,0>>, 6)),
+    {invalid_length,_} = dec_error('S', sub(<<40,16#80,1,1,255,0,0>>, 5)),
 
     %% A primitive must not be encoded with an indefinite length.
-    {error,{asn1,{invalid_length,_}}} =
-	(catch 'Constructed':decode('OS', <<4,128,4,3,97,98,99,0,0>>)),
+    {invalid_length,_} = dec_error('OS', <<4,128,4,3,97,98,99,0,0>>),
     ok.
 
+dec_error(T, Bin) ->
+    {error,{asn1,{Reason,Stk}}} = 'Constructed':decode(T, Bin),
+    [{_,_,_,_}|_] = Stk,
+    Reason.
+
 sub(Bin, Bytes) ->
     <<B:Bytes/binary,_/binary>> = Bin,
     B.
diff --git a/lib/asn1/test/testChoPrim.erl b/lib/asn1/test/testChoPrim.erl
index 573c482f2..61b6ab2d0 100644
--- a/lib/asn1/test/testChoPrim.erl
+++ b/lib/asn1/test/testChoPrim.erl
@@ -31,10 +31,10 @@ bool(Rules) ->
     roundtrip('ChoCon', {int2,233}),
     case Rules of
 	ber ->
-	    {error,{asn1,{invalid_choice_type,wrong}}} =
-		(catch 'ChoPrim':encode('ChoCon', {wrong,233})),
-	    {error,{asn1,{invalid_choice_tag,_WrongTag}}} =
-		(catch 'ChoPrim':decode('ChoCon', <<131,2,0,233>>));
+	    {error,{asn1,{{invalid_choice_type,wrong},[_|_]}}} =
+                 (catch 'ChoPrim':encode('ChoCon', {wrong,233})),
+	    {error,{asn1,{{invalid_choice_tag,_WrongTag},[_|_]}}} =
+                 (catch 'ChoPrim':decode('ChoCon', <<131,2,0,233>>));
 	per ->
 	    ok;
 	uper ->
diff --git a/lib/asn1/test/testInfObjectClass.erl b/lib/asn1/test/testInfObjectClass.erl
index 560986fac..540407fa5 100644
--- a/lib/asn1/test/testInfObjectClass.erl
+++ b/lib/asn1/test/testInfObjectClass.erl
@@ -33,19 +33,29 @@ main(Rule) ->
     roundtrip('Seq', Val),
     
     %% OTP-5783
-    {error,{asn1,{'Type not compatible with table constraint',
-		  {component,'ArgumentType'},
-		  {value,_},_}}} = 'InfClass':encode('Seq', {'Seq',12,13,1}),
+    {'Type not compatible with table constraint',
+     {component,'ArgumentType'},
+     {value,_},_} = enc_error('Seq', {'Seq',12,13,1}),
     Bytes2 = case Rule of
 		 ber ->
 		     <<48,9,2,1,12,2,1,11,2,1,1>>;
 		 _ ->
 		     <<1,12,1,11,1,1>>
 	     end,
-    {error,{asn1,{'Type not compatible with table constraint',
-		  {{component,_},
-		   {value,_B},_}}}} = 'InfClass':decode('Seq', Bytes2),
+    {'Type not compatible with table constraint',
+     {{component,_},
+      {value,_B},_}} = dec_error('Seq', Bytes2),
     ok.
 
 roundtrip(T, V) ->
     asn1_test_lib:roundtrip('InfClass', T, V).
+
+enc_error(T, V) ->
+    {error,{asn1,{Reason,Stk}}} = 'InfClass':encode(T, V),
+    [{_,_,_,_}|_] = Stk,
+    Reason.
+
+dec_error(T, Bin) ->
+    {error,{asn1,{Reason,Stk}}} = 'InfClass':decode(T, Bin),
+    [{_,_,_,_}|_] = Stk,
+    Reason.
diff --git a/lib/asn1/test/testPrim.erl b/lib/asn1/test/testPrim.erl
index 14546ff41..b2933dfab 100644
--- a/lib/asn1/test/testPrim.erl
+++ b/lib/asn1/test/testPrim.erl
@@ -195,7 +195,8 @@ roundtrip(Type, Value, ExpectedValue) ->
 enc_error(T, V) ->
     case get(no_ok_wrapper) of
 	false ->
-	    {error,{asn1,Reason}} = 'Prim':encode(T, V),
+	    {error,{asn1,{Reason,Stk}}} = 'Prim':encode(T, V),
+            [{_,_,_,_}|_] = Stk,
             Reason;
 	true ->
 	    try 'Prim':encode(T, V) of
-- 
2.12.0

Places

File 2431-encode-decode-Include-the-stack-trace-in-error-retur.patch of Package erlang (Revision d448831e03f614ee0c49b24ade3a1e91)

Places