ExHammer / hammer

An Elixir rate-limiter with pluggable backends
https://hexdocs.pm/hammer/
MIT License
738 stars 42 forks source link

Crash on Application startup with `String.to_existing_atom ` #85

Closed arjan closed 7 months ago

arjan commented 7 months ago

Hammer.Supervisor fails to start because it assumes the hammer backend names already exist in the atom pool.

Easily reproducible with running the following in iex:

Application.put_env(:hammer, :backend, [default: {Hammer.Backend.ETS, [expiry_ms: 10]}])
Mix.install([:hammer])

Crashes in the following way:

20:58:30.827 [notice] Application hammer exited: Hammer.Application.start(:normal, []) returned an error: an exception was raised:
    ** (ArgumentError) errors were found at the given arguments:

  * 1st argument: not an already existing atom

        :erlang.binary_to_existing_atom("hammer_backend_default_pool", :utf8)
        (hammer 6.2.0) lib/hammer/supervisor.ex:30: anonymous fn/1 in Hammer.Supervisor.init/1
        (elixir 1.15.4) lib/enum.ex:1693: Enum."-map/2-lists^map/1-1-"/2
        (hammer 6.2.0) lib/hammer/supervisor.ex:28: Hammer.Supervisor.init/1
        (stdlib 5.0.2) supervisor.erl:330: :supervisor.init/1
        (stdlib 5.0.2) gen_server.erl:962: :gen_server.init_it/2
        (stdlib 5.0.2) gen_server.erl:917: :gen_server.init_it/6
        (stdlib 5.0.2) proc_lib.erl:241: :proc_lib.init_p_do_apply/3
epinault commented 7 months ago

hello! thanks for the report. What version oif Elixir and Erlang OTP are you running?

arjan commented 7 months ago

Otp 26, elixir 1.15

epinault commented 7 months ago

I see, yea I have not done any testing around that yet. it s related to the change they added in 1.15 . https://hexdocs.pm/elixir/1.15/changelog.html#potential-incompatibilities . Can you try this in your project and see if that helps?

njwest commented 7 months ago

+1, getting the same error on a fresh install of Hammer 6.2 on Elixir 1.15.6 and OTP 26

njwest commented 7 months ago

I see, yea I have not done any testing around that yet. it s related to the change they added in 1.15 . https://hexdocs.pm/elixir/1.15/changelog.html#potential-incompatibilities . Can you try this in your project and see if that helps?

Tried this, did not help. Looking at the code I don't see where these atoms are getting instantiated prior to String.to_existing_atom/1 call

njwest commented 7 months ago

Hammer.Supervisor fails to start because it assumes the hammer backend names already exist in the atom pool.

@arjan I am (edit:)ALMOST able to bypass this bug by changing my hammer backend: config value from a list to a single tuple like so:

config :hammer,
  backend: {
      Hammer.Backend.ETS,
      [
        expiry_ms: 60_000 * 60 * 2,
        cleanup_interval_ms: 300_000 * 2
      ]
    }

because single-backend config uses a single static default pool atom, HOWEVER when I do this I run into the following error when trying to call Hammer:

** (exit) an exception was raised:
    ** (ArgumentError) errors were found at the given arguments:

  * 1st argument: not an already existing atom

        :erlang.binary_to_existing_atom("hammer_backend_ets_pool", :utf8)
        (hammer 6.2.0) lib/hammer/utils.ex:9: Hammer.Utils.pool_name/1
        (hammer 6.2.0) lib/hammer.ex:255: Hammer.call_backend/3
        (hammer 6.2.0) lib/hammer.ex:51: Hammer.check_rate/4

new issue opened #88