String.to_existing_atom failing in utils.ex

[njwest]

Describe the bug When calling Hammer functions with a single backend, NOT a list of backends, the call fails with:

** (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

** Provide the following details

• Elixir version (elixir -v): Elixir 1.15.6
• Erlang version (erl -v): Erlang/OTP 26
• Operating system: macOS 14.2

Expected behavior I expect Hammer.check_rate/4 to not result in an error

Actual behavior See above

[njwest]

def pool_name(name) do
   String.to_existing_atom("hammer_backend_#{name}_pool")
end

@epinault is there a reason why this needs to be to_existing_atom/1? I can throw up a quick PR similar to #87 that just changes this to to_atom/1 but I don't wanna do that if there is an ideological resistance to to_atom/1

[epinault]

I think it s to avoid some risk of Atom during configuration but we can make it to_atom for now if you open an MR

https://hexdocs.pm/credo/Credo.Check.Warning.UnsafeToAtom.html

[njwest]

I think it s to avoid some risk of Atom during configuration but we can make it to_atom for now if you open an MR

https://hexdocs.pm/credo/Credo.Check.Warning.UnsafeToAtom.html

@epinault a fix for this has been added to PR #87

Edit: Re: the UnsafeToAtom check, String.to_atom/1 can be dangerous if it is getting called a lot, e.g. if you have a user input that creates atoms on the backend; this is not a concern with configuration, which is set by us devs and isn't exposed to users.

IMHO we have a use case in Hammer where to_atom should be used, as to_existing_atom doesn't work in this implementation anymore.

Also, the credo check itself is tagged :controversial, likely because some folks think to_atom is always bad when it is actually fine if used responsibly :P

from the credo link:

[wsbinette]

+1 also need this, can't deploy new project without it. 🙏🏼

[epinault]

Hello, I am have released a new version 6.2.1. I am unfortunately traveling for another few days and have limited access so hopefully that will unblock you

[njwest]

Thank you @epinault !!! 🙏 that solved it for me