denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
98.26k stars 5.41k forks source link

Implement Happy Eyeballs for TCP connections #25661

Open lucacasonato opened 2 months ago

lucacasonato commented 2 months ago

Right now we generally prefer IPv6 to IPv4 addresses when connecting to services. This unfortunately causes users to run into "connection failed" errors when their service does not actually listen on IPv6 (even though the DNS resolves to an IPv6 address).

Happy Eyeballs fixes this. It establishes connections to multiple addresses in parallel and uses the first one that connects.

seanmonstar commented 2 months ago

As on potential option, the HttpConnector in hyper-util provides happy eyeballs, and it returns a TcpStream (just pluck it out of the TokioIo wrapper). It could be as easy as replacing the call of TcpStream::connect with a http_connector.call().

jarlah commented 1 month ago

This is also an issue with testcontainers-node, where it fails to resolve docker client

https://github.com/testcontainers/testcontainers-node/blob/main/packages/testcontainers/src/container-runtime/clients/client.ts#L64

due to same reason

Fetching Docker info...
Container runtime strategy "UnixSocketStrategy" does not work: "TypeError: error sending request for url (http://localhost/info): client error (Connect): tcp connect error: Connection refused (os error 111): Connection refused (os error 111)"
TypeError: error sending request for url (http://localhost/info): client error (Connect): tcp connect error: Connection refused (os error 111): Connection refused (os error 111)
    at async node:http:313:21

i ported the testcontainers package to deno and made a simple deno test to make a generic container like this:

import { assertEquals } from "@std/assert";
import { add } from "./main.ts";
import { GenericContainer } from "./src/index.ts";

Deno.test(async function addTest() {
  assertEquals(add(2, 3), 5);
  await new GenericContainer("redis")
      .withExposedPorts(6379)
      .start();

});
jarlah commented 1 month ago

I could try to make a stab on this, if no one has planned it yet. EDIT: wow this is the first opensource project i have ever managed to get up and running in no time in vscode, just wow

jarlah commented 1 month ago

im beginning to think why use localhost at all, why not just use 127.0.0.1 :man_shrugging: i think there has been several debates about localhost vs 127.0.0.1 on the internet, saying localhost is not stable. Its system dependent.

btw, poking a bit into the ext/net, nothing particularly stands out and i think i would need some serious pointers if i were to do anything with that module in terms of fixing this issue ;)

made a test though that just proves the point, that localhost returns both ipv6 and ipv4.

  #[test]
  fn resolve_addr_localhost_sync() {
    let expected = vec![
      SocketAddr::V6(SocketAddrV6::new(
        Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1),
        80,
        0,
        0
      )),
      SocketAddr::V4(SocketAddrV4::new(
        Ipv4Addr::new(127, 0, 0, 1),
        80,
      ))
    ];
    let actual = resolve_addr_sync("localhost", 80)
      .unwrap()
      .collect::<Vec<_>>();
    assert_eq!(actual, expected);
  }