search

elixir-mint / mint

Functional HTTP client for Elixir with support for HTTP/1 and HTTP/2 🌱
Apache License 2.0
1.36k stars 106 forks source link

Network errors doesn't surface correctly in IPv6-only network #395

Closed danschultzer closed 4 months ago

danschultzer commented 1 year ago

Is there a good reason to have this default IPv4 fallback when explicitly setting IPv6?

https://github.com/elixir-mint/mint/blob/ca4e975214bfbb7627f6b52b3a5ea2a6b97964df/lib/mint/core/transport/ssl.ex#L333-L339

I'm using Mint (via Finch) in an an IPv6-only network, but got timeout errors. It actually seems to originate from a cert error, which I discovered by testing the URL in local with dual-stack setup. With the above fallback the TLS error is not surfaced.

The fallback is necessary in a dual-stack setup, but I think making the setting explicit would be better:

# IPv6 only
[inet: :inet6]

# IPv4 only (this is the current default)
[inet: :inet]

# IPv6 with IPv4 fallback
[inet: [:inet6, :inet]]

# IPv4 with IPv6 fallback
[inet: [:inet, :inet6]]

Using :inet as the key here, not sure what would be the better name (I've used :ipfamily in the past). Also a thing to note, most browsers + curl implements the happy eyeballs algorithm, which might be the best default since we have to deal with dual-stack for a long time coming.

I would be happy to put together a PR if the above approach makes sense!

whatyouhide commented 1 year ago

TIL about Happy Eyeballs! 👀

Would love if you could implement that in a PR, if you have the time? 🙃

ericmj commented 4 months ago

Introducing a new :inet would be difficult because it means we would have to deprecate :inet6 and determine a priority for each option.

Instead I would suggest to add a Keyword.get(opts, :inet, true) check to determine if we should do IPv4 fallback.

whatyouhide commented 4 months ago

@ericmj sounds good to me.

ericmj commented 4 months ago

Great, I'll send a PR.

ruslandoga commented 2 weeks ago

👋

Sorry for posting in an old issue, but I wonder if opening a connection with :gen_tcp.connect first and then doing :ssl.connect(tcp_socket, ...) would help to surface the errors better in inet6: true case? Kind of like here for SMTP STARTTLS: https://github.com/ruslandoga/mua/blob/c8e82f0ac921198b4d56f019498906a7707a59c0/lib/mua.ex#L311

I think it was also suggested in the original inet6: true PR: https://github.com/elixir-mint/mint/pull/270#issuecomment-678372248