mozilla-services / ip-reputation-js-client

A node JS client to the iprepd IP reputation service
Mozilla Public License 2.0
5 stars 8 forks source link

[bug 1475698] apply timeout to connection and DNS lookup timeouts for new connections #23

Closed g-k closed 6 years ago

g-k commented 6 years ago

The current timeout param only applies to waiting between reads and writes on the socket.

New connections that do not reuse an existing TCP connection, will fire lookup and connect events, but potentially hit the OS connection (and probably DNS lookup) timeouts.

per irc chat w/ @ameihm0912:

one of the tests we did was to totally firewall off the iprepd elb if we do that, the request ends up taking 30s or so (reverts to default OS timeout per requests documentation)

The docs say:

Note that if the underlying TCP connection cannot be established, the OS-wide TCP connection timeout will overrule the timeout option (the default in Linux can be anywhere from 20-120 seconds).

Setting this at the OS-level with net.ipv4.tcp_syn_retries is not desirable since we want other FxA connections to use the default timeout.

The easiest solution might be to:

Additional info in: https://github.com/request/request/tree/master#timeouts The commit adding connection timeout tracking is: https://github.com/request/request/commit/ee4cfbbc66dc1bfb7481e79ab7ce51d4e0ba9038

g-k commented 6 years ago

This should actually be pretty straightforward with: http://bluebirdjs.com/docs/api/timeout.html