wooga / eredis

Erlang Redis client
MIT License
628 stars 279 forks source link

ipv6 support cause issues in some cases #125

Open arcusfelis opened 3 years ago

arcusfelis commented 3 years ago

Hi, sometimes we have Docker configured with ipv6 resolving, but Redis server is only listening on ipv4. So redis_client would resolve ipv6, try to connect and fail with a pretty non-descriptive eaddrnotavail. After that people would try to do telnet localhost 6379 and it would work. They would try to do gen_tcp:connect("localhost", 6379, []). and it would work. But eredis would not work.

eredis would basically do that behind the scene:

gen_tcp:connect({0,0,0,0,0,0,0,1}, 6379, []) -> {error,eaddrnotavail}

I doubt this code is even needed:

get_addr({local, Path}) ->
    {ok, {local, {local, Path}}};
get_addr(Hostname) ->
    case inet:parse_address(Hostname) of
        {ok, {_,_,_,_} = Addr} ->         {ok, {inet, Addr}};
        {ok, {_,_,_,_,_,_,_,_} = Addr} -> {ok, {inet6, Addr}};
        {error, einval} ->
            case inet:getaddr(Hostname, inet6) of
                 {error, _} ->
                     case inet:getaddr(Hostname, inet) of
                         {ok, Addr}-> {ok, {inet, Addr}};
                         {error, _} = Res -> Res
                     end;
                 {ok, Addr} -> {ok, {inet6, Addr}}
            end
    end.

It would be much cleaner solution to just use gen_tcp:connect/3 logic and pass State#state.host as a first argument, without resolving it first. If people really need to use ipv6, they could pass inet6 inside of SocketOpts.

arcusfelis commented 3 years ago

Alternatively, we can try both ipv6 and ipv4, like in this commit: https://github.com/arcusfelis/eredis/commit/96a3848af1d54cadb09700ef8b1014e92183587d

But I doubt the code compexity introduced in this case would justify that. We already have some complex code with not expected behaviour with get_addr/1.