PolymorphicEmbed.types not working if the module doesn't get recompiled

[dottorblaster]

First of all, thanks for the amazing work on this library! :smile:

I think I stepped into a tiny bug when it comes to the types function, which I'm heavily relying upon for some reflection into some generated code of my own.

Given this module content (all in one file just for the sake of reproduction):

defmodule MyApp.Channel.Email do
  use Ecto.Schema
  import Ecto.Changeset

  @primary_key false

  embedded_schema do
    field :address, :string
    field :confirmed, :boolean
  end

  def changeset(email, params) do
    email
    |> cast(params, ~w(address confirmed)a)
    |> validate_required(:address)
    |> validate_length(:address, min: 4)
  end
end

defmodule MyApp.Channel.SMS do
  use Ecto.Schema

  @primary_key false

  embedded_schema do
    field :number, :string
  end
end

defmodule PolymorphicEmbedBug do
  use Ecto.Schema
  import Ecto.Changeset
  import PolymorphicEmbed

  schema "reminders" do
    field :date, :utc_datetime
    field :text, :string

    polymorphic_embeds_one :channel,
      types: [
        sms: MyApp.Channel.SMS,
        email: MyApp.Channel.Email
      ],
      on_type_not_found: :raise,
      on_replace: :update
  end

  def changeset(struct, values) do
    struct
    |> cast(values, [:date, :text])
    |> cast_polymorphic_embed(:channel, required: true)
    |> validate_required(:date)
  end
end

I get this behavior when I run iex for the first time, having the module being compiled:

9:16:02 › iex -S mix
Erlang/OTP 25 [erts-13.0.3] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

==> decimal
Compiling 4 files (.ex)
Generated decimal app
===> Analyzing applications...
===> Compiling telemetry
==> jason
Compiling 10 files (.ex)
Generated jason app
==> ecto
Compiling 56 files (.ex)
Generated ecto app
==> polymorphic_embed
Compiling 2 files (.ex)
Generated polymorphic_embed app
==> polymorphic_embed_bug
Compiling 1 file (.ex)
Generated polymorphic_embed_bug app
Interactive Elixir (1.13.2) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> PolymorphicEmbed.types(PolymorphicEmbedBug, :channel)
[:sms, :email]

But if I just stop iex (hence the BEAM) and restart it again with no changes, the atoms related to the polymorphic type don't get initialized and I get an error when I try to .types those, because the .types function uses String.to_existing_atom, which comes from the correct mindset that one shouldn't flood the BEAM with new atoms, I guess.

9:16:53 › iex -S mix
Erlang/OTP 25 [erts-13.0.3] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

Interactive Elixir (1.13.2) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> PolymorphicEmbed.types(PolymorphicEmbedBug, :channel)                                                                       
** (ArgumentError) errors were found at the given arguments:

  * 1st argument: not an already existing atom

    :erlang.binary_to_existing_atom("sms", :utf8)
    (polymorphic_embed 3.0.0) lib/polymorphic_embed.ex:370: anonymous fn/1 in PolymorphicEmbed.types/2
    (elixir 1.13.2) lib/enum.ex:1593: Enum."-map/2-lists^map/1-0-"/2

I was considering giving you the hint to just use String.to_atom for this, but actually I think it's correct not spamming the BEAM with new atoms everytime. What do you think? Is there some way one could work around this, or is a fix possible?

[mathieuprog]

1. You can reproduce the problem every time consistently?
2. What if you replace:

    polymorphic_embeds_one :channel,
      types: [
        sms: MyApp.Channel.SMS,
        email: MyApp.Channel.Email
      ],
      on_type_not_found: :raise,
      on_replace: :update

by

    field :channel, PolymorphicEmbed,
      types: [
        sms: MyApp.Channel.SMS,
        email: MyApp.Channel.Email
      ],
      on_type_not_found: :raise,
      on_replace: :update

[dottorblaster]

@mathieuprog thanks for the immediate answer!

1. Yes. All it takes is starting iex with no code modifications to the module, causing no recompilation
2. Nothing changes. It works the first time, then it will stop unless I cause a module recompilation with some code change. Of course I'm worried of the fact that I'm distributing my app through an Elixir release so in production I guess this won't work at all :frowning_face:

I swear, all it takes is pasting those modules into a file and having Ecto and PolymorphicEmbed into the deps list :sweat_smile:. I can make a public repo out of it you want to personally test.

[mathieuprog]

Yeah I'm able to test that out later. Full-time jobs suck your time:(

What if you add the atoms you use in :types at the top?

:sms
:email

[dottorblaster]

Same behavior I'm afraid :/

[dottorblaster]

Yeah I'm able to test that out later. Full-time jobs suck your time:(

Don't worry, I can relate :smile:

[mathieuprog]

Are you saying that you have an atom :foo, anywhere, and that when you run :erlang.binary_to_existing_atom("foo", :utf8) an error is raised?

[dottorblaster]

It's exactly what I'm saying :sweat_smile:

[mathieuprog]

Ok so the problem is not specific to PolymorphicEmbed?

1. You have an atom :foo (not related to PolymorphicEmbed)
2. You call :erlang.binary_to_existing_atom("foo", :utf8) (not related to PolymorphicEmbed)
3. An error is raised (not related to PolymorphicEmbed)

[dottorblaster]

Yes, the specific problem with PolymorphicEmbed is that due to this behavior I can't invoke the types function :/

[mathieuprog]

Ok so why do we have this behavior? It can create you problems with other libs similarly. I think it's worth asking the Elixir community. The cause? The impact? What do you think? I can't change the code for something we do not grasp, you know that 😛

[dottorblaster]

Ok, I'll ask around and report that to you, thanks :sweat_smile:

[mathieuprog]

Thanks. Nice try tho 😝

[mathieuprog]

Do you have any update on why there is this behavior?

[mathieuprog]

ping @dottorblaster

[dottorblaster]

Hi! I actually have updates on this, but it's quite complex and I think I'll need some time to jot something down. Forgive me :bow:

[mathieuprog]

Nice. Did you ask the community? Are your interactions/questions public? Anyway, I'll wait for your feedback:)

[dottorblaster]

Well, before it gets deleted because of the retention policy you can see my messages on Elixir's Slack space https://elixir-lang.slack.com/archives/C03EPRA3B/p1661862381672099

Anyway, the problem is that when the module gets compiled, the BEAM has the atom inside and when the .types(:field) function gets invoked String.to_existing_atom() works correctly. If you turn off the BEAM program and restart it again, the bytecode doesn't actually have the atom inside because it only gets created while compiling, so basically what happens is that to_existing_atom can't be invoked anymore.

I inspected the bytecode generated by an Elixir module having a :foo atom inside, and after building the atom is simply gone.

[mathieuprog]

It's hard to understand for me lol. In your opinion, do you think there is a problem in the library?

In the module you added under defmodule

:sms
:email

What if you add those atoms in a function:

def channel_types() do
  [:sms, :email]
end

(no idea what I'm talking about, but you can try?)

[mathieuprog]

ping @dottorblaster

[dottorblaster]

Sorry, tough times.

Actually it works if I define such a function!

do you think there is a problem in the library?

I think so, because right now PolymorphicEmbed.types() can't really be used at all :frowning_face: defining such a function is a solution. Anyway, I can create a repo with the faulty code if you want to tinker with it :smile:

[mathieuprog]

Thank you for your feedback! I changed the issue to bug. I asked you to add a function returning the atoms as I read from your Slack replies someone mentioning the atoms get "absorbed", so I thought that the atoms are not "persisted" because they are only used for some compile-time metaprogramming then discarded. So if you added the list of atoms at the module level, they are only used at compile-time. However when returned from a function, they are needed for runtime. Something along those lines. Is just an idea but as you say it works now, maybe this is right?

So what we should do is return the atoms in the init somehow. You can see here I convert them to string because I found them easier to manipulate in strings: https://github.com/mathieuprog/polymorphic_embed/blob/v3.0.2/lib/polymorphic_embed.ex#L42 Maybe that is the problem.

Can you try to just add the opts in the map returned from init/1? So:

    %{
      opts: opts,
      default: Keyword.get(opts, :default, nil),
      on_replace: Keyword.fetch!(opts, :on_replace),
      on_type_not_found: Keyword.get(opts, :on_type_not_found, :changeset_error),
      type_field: Keyword.get(opts, :type_field, :__type__) |> to_string(),
      types_metadata: types_metadata
    }

you can just add it into your deps and then do mix deps.recompile. Just add a dummy log to make sure there's a change.

[dottorblaster]

@mathieuprog your guessing was right, now the error is gone :+1:

However when returned from a function, they are needed for runtime

I think you are actually right, from what I saw this is a correct statement. Basically using decompile we can see that the original version misbehaves because the module doesn't have the atom into its bytecode:

blaster at gandalf in [~/Desktop/projects/polymorphic_embed_bug]   
9:12:11 › iex -S mix
Erlang/OTP 25 [erts-13.0.4] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

Interactive Elixir (1.14.0) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> PolymorphicEmbed.types(PolymorphicEmbedBug, :channel)
** (ArgumentError) errors were found at the given arguments:

  * 1st argument: not an already existing atom

    :erlang.binary_to_existing_atom("sms", :utf8)
    (polymorphic_embed 3.0.0) lib/polymorphic_embed.ex:370: anonymous fn/1 in PolymorphicEmbed.types/2
    (elixir 1.14.0) lib/enum.ex:1658: Enum."-map/2-lists^map/1-0-"/2
    iex:1: (file)
iex(1)> 
BREAK: (a)bort (A)bort with dump (c)ontinue (p)roc info (i)nfo
       (l)oaded (v)ersion (k)ill (D)b-tables (d)istribution
^C%                                                                                                                                          
blaster at gandalf in [~/Desktop/projects/polymorphic_embed_bug]   
9:12:17 › mix decompile PolymorphicEmbedBug --to core

blaster at gandalf in [~/Desktop/projects/polymorphic_embed_bug]   
9:12:22 › cat Elixir.PolymorphicEmbedBug.core | grep sms

While adding opts: opts to the map returned by init makes possible for the atom to be actually "persisted" into the bytecode:

blaster at gandalf in [~/Desktop/projects/polymorphic_embed_bug]   
9:14:53 › iex -S mix
Erlang/OTP 25 [erts-13.0.4] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

Interactive Elixir (1.14.0) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> PolymorphicEmbed.types(PolymorphicEmbedBug, :channel)
[:sms, :email]
iex(2)> 
BREAK: (a)bort (A)bort with dump (c)ontinue (p)roc info (i)nfo
       (l)oaded (v)ersion (k)ill (D)b-tables (d)istribution
^C%                                                                                                                                          
blaster at gandalf in [~/Desktop/projects/polymorphic_embed_bug]   
9:15:02 › mix decompile PolymorphicEmbedBug --to core

blaster at gandalf in [~/Desktop/projects/polymorphic_embed_bug]   
9:15:08 › cat Elixir.PolymorphicEmbedBug.core | grep sms
	 ~{'channel'=>{'parameterized','Elixir.PolymorphicEmbed',~{'default'=>'nil','on_replace'=>'update','on_type_not_found'=>'raise','opts'=>[{'types',[{'sms','Elixir.MyApp.Channel.SMS'}|[{'email','Elixir.MyApp.Channel.Email'}]]}|[{'on_type_not_found','raise'}|[{'on_replace','update'}|[{'field','channel'}|[{'schema','Elixir.PolymorphicEmbedBug'}]]]]],'type_field'=>#{#<95>(8,1,'integer',['unsigned'|['big']]),
		 ~{'channel'=>{'channel',{'parameterized','Elixir.PolymorphicEmbed',~{'default'=>'nil','on_replace'=>'update','on_type_not_found'=>'raise','opts'=>[{'types',[{'sms','Elixir.MyApp.Channel.SMS'}|[{'email','Elixir.MyApp.Channel.Email'}]]}|[{'on_type_not_found','raise'}|[{'on_replace','update'}|[{'field','channel'}|[{'schema','Elixir.PolymorphicEmbedBug'}]]]]],'type_field'=>#{#<95>(8,1,'integer',['unsigned'|['big']]),
		 [{'id','id'}|[{'date','utc_datetime'}|[{'text','string'}|[{'channel',{'parameterized','Elixir.PolymorphicEmbed',~{'default'=>'nil','on_replace'=>'update','on_type_not_found'=>'raise','opts'=>[{'types',[{'sms','Elixir.MyApp.Channel.SMS'}|[{'email','Elixir.MyApp.Channel.Email'}]]}|[{'on_type_not_found','raise'}|[{'on_replace','update'}|[{'field','channel'}|[{'schema','Elixir.PolymorphicEmbedBug'}]]]]],'type_field'=>#{#<95>(8,1,'integer',['unsigned'|['big']]),
		 {'parameterized','Elixir.PolymorphicEmbed',~{'default'=>'nil','on_replace'=>'update','on_type_not_found'=>'raise','opts'=>[{'types',[{'sms','Elixir.MyApp.Channel.SMS'}|[{'email','Elixir.MyApp.Channel.Email'}]]}|[{'on_type_not_found','raise'}|[{'on_replace','update'}|[{'field','channel'}|[{'schema','Elixir.PolymorphicEmbedBug'}]]]]],'type_field'=>#{#<95>(8,1,'integer',['unsigned'|['big']]),

Pretty cool, huh? :smile:

Now, we've got a fix but actually I wouldn't know how to test that in a proper automated way. I was trying to run the tests, but they require a running database :sweat_smile: and I don't find any docker-compose.yml or something like that. Set the testing aside, this is actually a fix :+1:

[mathieuprog]

All the credits go to LostKobrakai though, I've just repeated what he found out, his knowledge, and also he suggested to return the atoms.

I think this doesn't require a test but it does require a comment in the code with a reference to this issue. I'll add this soon.

[mathieuprog]

Could you test with the update on master?

{:polymorphic_embed, git: "git://github.com/mathieuprog/polymorphic_embed.git",
        branch: "master",
        override: true},

I'll create a release after you confirm the bug is gone.

[dottorblaster]

@mathieuprog looks like the bug is gone :+1:

[mathieuprog]

Fixed in 3.0.3 https://elixirforum.com/t/31951/11