Add timeout named argument to 'Socket.connect', etc.

[DartBot]

This issue was originally filed by ef...@gmail.com

---

What steps will reproduce the problem?
import 'dart:io';

main() {
  var client = new HttpClient();
  
  client.getUrl(Uri.parse('http://8.8.8.9')) // host without response
  .timeout(const Duration(seconds: 5))
  .then((HttpClientRequest request) {
    print("hell no.");
  })
  .catchError((e) => print(e.toString()))
  .whenComplete(() => client.close(force: true));
}

What is the expected output? What do you see instead?

• expected: 5 seconds, TimeoutException, exit
• instead: 5 seconds, TimeoutException, 2 minutes, exit

What version of the product are you using? On what operating system?

• 64 bit linux (Ubuntu)

[sethladd]

Added Library-IO, Area-Library, Triaged labels.

[andersjohnsen]

Calling 'timeout' on a future does not stop the original operation that completes the future. A timeout of 2 minute is default on most systems. The real way to 'fix' this would be to add a named argument 'timeout' to Socket.connect & others, that would override the default 2 minute timeout.

---

Removed Type-Defect label.
Added Type-Enhancement label.
Changed the title to: "Add timeout named argument to 'Socket.connect', etc.".

[DartBot]

This comment was originally written by ef...@gmail.com

---

Sure it would help, but would it be better to cancel socket operations by client.close() or some equivalent? Closing socket/client with command is actualy more controllable in "dart way" then "static" timeout (and without changing api for now).

(But setting socket timeout should added in future ofcourse...)

[DartBot]

This comment was originally written by greg.lo...@gmail.com

---

This would be great to have for use in the postgresql library. These 2 min hangs are a real pain when running unittests. But more importantly I'd like to be able to set a lower timeout in the connection pool.

[floitschG]

@zanderso : this just came up on gitter.

What's your opinion?

We could wrap all futures that are returned by socket operations and back-propagate the timeout, but that feels expensive. It would provide one of the nicest APIs' though.

Alternatively, we could add named timeout arguments to the relevant functions.

Another approach would be to set the timeout time globally. That would make unit-testing easier/faster.

Finally, we could have a method that actively cancels any running operation.

[sircco]

@floitschG

it would be nice if timeout could be differentiated by send/recv timeout via setsockopt or similar method

here is what python does
some high level functions expose socket via instance._get_socket()
then you use socket.settimeout(n) for that socket

also relevant functions have timeout param. i.e.
https://docs.python.org/3/library/http.client.html
https://docs.python.org/2/library/smtplib.html

similar stuff is in golang, with Conn interface with dial timeout and deadlines
https://golang.org/pkg/net/

as i see from docs dart's setsockopt is very limiting

[zanderso]

Taking a brief look, I think the approach we should consider is wiring up sendTimeout and recvTimeout named arguments to connect (on Socket) and bind (on ServerSocket for sockets we get back from accept) to setsockopt with SO_RCVTIMEO and SO_SNDTIMEO.

If there's demand for it going forward, we can consider providing a more general API for accessing socket options.

[bkonyi]

Would it maybe just make more sense to add a SocketOptions class and have connect and bind consume that? That way we won't have to change the interface for Socket and ServerSocket if we do eventually decide to allow for additional configuration options.

[zanderso]

@bkonyi sgtm.

@floitschG Thoughts about @bkonyi's suggestion?

[floitschG]

Normally, named arguments scale quite nicely. What would be the advantage of having a SocketOptions class?

[allComputableThings]

Is there any plan to add a "socket timeout" (a timeout associated with send/recv), instead of just the connect timeout? (AFAIK 86fdbde only added add the connect timeout).

[bkonyi]

At this point, I don't think so. It's probably worth filing a feature request for though.