AVRO_REQUIRED in eravro.hrl prevents record from being extracted by elixir

[LostKobrakai]

https://github.com/klarna/erlavro/blob/c55e55238ed879642c65d00c8105e498b4acc8db/include/erlavro.hrl#L51

This line seems to cause :epp to error for all records using it and therefore the only record extractable by elixir is :avro_value.

[mikpe]

Please elaborate. This is Erlang, and it obviously works with Erlang.

[LostKobrakai]

Elixir extracts records by parsing the .hrl with :epp.parse_file at compile time to get the record's field details. From that it dynamically defines some elixir macros to make working with records a bit less verbose over just handling the tuple form directly.

With the current error in AVRO_REQUIRED any record using it won't return the records abstract form in :epp.parse_file's results, but just an error, making it impossible for elixir to extract the data it needs.

[mikpe]

I can't reproduce. If I simulate a bad record with say

> cat q.erl
-module(q).
-export([foo/0]).
-record(bar, {x = erlang:error(gazonk)}).
foo() -> #bar{}.

then not only does it compile but epp:parse_file/2 returns valid-looking abstract form:

1> c("q.erl").
{ok,q}
2> epp:parse_file("q.erl", []).
{ok,[{attribute,1,file,{"q.erl",1}},
     {attribute,1,module,q},
     {attribute,2,export,[{foo,0}]},
     {attribute,3,record,
                {bar,[{record_field,3,
                                    {atom,3,x},
                                    {call,3,
                                          {remote,3,{atom,3,erlang},{atom,3,error}},
                                          [{atom,3,gazonk}]}}]}},
     {function,4,foo,0,[{clause,4,[],[],[{record,4,bar,[]}]}]},
     {eof,5}]}
3> 

So Elixir must be doing something odd, and possibly broken. @zmstone -- you worked on this before, do you have any idea if erlavro or Elixir is broken here?

[zmstone]

I’m not quite familiar with elixir record extraction. Judging by the example below, it actually returns not just the fields, but (default?) values too. https://hexdocs.pm/elixir/Record.html#extract/2-examples

[LostKobrakai]

This is the result for parsing erlavro.hrl (which is what elixir uses under the hood). As you can see only the avro_value record is present. While the errors correspond to the lines, which use ?AVRO_REQUIRED. I don't see what elixir could change here, given this is the results it gets from :epp.parse_file/2.

iex(1)> :epp.parse_file('./include/erlavro.hrl', [])
{:ok,
 [
   {:attribute, 1, :file, {'./include/erlavro.hrl', 1}},
   {:error, {57, :epp, {:undefined, :MODULE, :none}}},
   {:error, {62, :epp, {:undefined, :MODULE, :none}}},
   {:error, {72, :epp, {:undefined, :MODULE, :none}}},
   {:error, {82, :epp, {:undefined, :MODULE, :none}}},
   {:error, {87, :epp, {:undefined, :MODULE, :none}}},
   {:error, {92, :epp, {:undefined, :MODULE, :none}}},
   {:error, {97, :epp, {:undefined, :MODULE, :none}}},
   {:attribute, 105, :record,
    {:avro_value,
     [
       {:typed_record_field, {:record_field, 106, {:atom, 106, :type}},
        {:remote_type, 106,
         [{:atom, 106, :avro}, {:atom, 106, :type_or_name}, []]}},
       {:typed_record_field, {:record_field, 107, {:atom, 107, :data}},
        {:remote_type, 107,
         [{:atom, 107, :avro}, {:atom, 107, :avro_value}, []]}}
     ]}},
   {:attribute, 110, :type,
    {:avro_value,
     {:type, 110, :union,
      [
        {:remote_type, 110,
         [{:atom, 110, :avro}, {:atom, 110, :canonicalized_value}, []]},
        {:type, 111, :record, [{:atom, 111, :avro_value}]},
        {:type, 112, :list,
         [{:type, 112, :record, [{:atom, 112, :avro_value}]}]},
        {:type, 113, :list,
         [
           {:type, 113, :tuple,
            [
              {:remote_type, 113,
               [{:atom, 113, :avro}, {:atom, 113, :name}, []]},
              {:type, 113, :record, [{:atom, 113, :avro_value}]}
            ]}
         ]},
        {:remote_type, 114, [{:atom, 114, :avro_map}, {:atom, 114, :data}, []]},
        {:type, 115, :tuple, [{:atom, 115, :json}, {:type, 115, :binary, []}]},
        {:type, 116, :tuple, [{:atom, 116, :binary}, {:type, 116, :binary, []}]}
      ]}, []}},
   {:attribute, 123, :type,
    {:avro_encoding,
     {:type, 123, :union,
      [{:atom, 123, :avro_json}, {:atom, 123, :avro_binary}]}, []}},
   {:eof, 140}
 ]}

P.S: Just tried it out in erl and had the same results.

[mikpe]

The issue is that ?MODULE is undefined when you're just pre-processing the .hrl file. I tried to work around that but it seems -ifdef(MODULE) doesn't actually work.

I don't think we need to embed our own location data in the exception, since the stack trace will include it anyway. So the fix would be to just remove the ?MODULE part. WDYT @zmstone ?

[mikpe]

The issue is that ?MODULE is undefined when you're just pre-processing the .hrl file. I tried to work around that but it seems -ifdef(MODULE) doesn't actually work.

I don't think we need to embed our own location data in the exception, since the stack trace will include it anyway. So the fix would be to just remove the ?MODULE part. WDYT @zmstone ?

If the location is deemed useful we could alternatively replace ?MODULE with ?FILE which does work when pre-processing a .hrl file.

[zmstone]

I don't think we need to embed our own location data in the exception, since the stack trace will include it anyway. So the fix would be to just remove the ?MODULE part. WDYT @zmstone ?

Agree.