Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Timeout functionality for http package #19831

Closed
DartBot opened this issue Jul 4, 2014 · 13 comments
Closed

add Timeout functionality for http package #19831

DartBot opened this issue Jul 4, 2014 · 13 comments
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented Jul 4, 2014

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


What steps will clearly show the issue / need for enhancement?

  1. when making http requests you usually want to timeout the request after a reasonable amount of time (usually in the region of seconds)
  2. it would be great to have away to specify timeout as Duration when making a http request via the http/browser client/http client classes.

What version of the product are you using? On what operating system?
Dart 1.5.1, http 0.11.1+1

@dgrove
Copy link
Contributor

dgrove commented Jul 7, 2014

Added Pkg-Http, Triaged labels.

@nex3
Copy link
Member

nex3 commented Jul 7, 2014

Probably the best way to support this would be to add a separate http_timeout_client package that exposes an HttpTimeoutClient that wraps an existing client.

@DartBot
Copy link
Author

DartBot commented Jul 7, 2014

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


Is there any reason why timeout couldn't be added as an optional parameter to the existing clients? I think the usage pattern would be more obvious this way than having to first create a client, then create another (HttpTimeoutClient) to wrap around it before you do a HTTP request.

@nex3
Copy link
Member

nex3 commented Jul 8, 2014

The http package is based on a composition model where each layer (that is, a class implementing http.Client) is very simple and has a well-defined purpose. This model makes it very easy to mix and match functionality, but one of the costs is that the API for a layer has to be very simple in order to keep them easy to implement. It would be bad if everyone implementing their own [http.Client] had to re-implement the timeout logic from scratch.

having to first create a client, then create another (HttpTimeoutClient) to wrap around it before you do a HTTP request.

The API wouldn't be that difficult. The constructor would look something like this:

    HttpTimeoutClient([HttpClient inner])
        : _inner = inner == null ? new HttpClient() : inner;

The user only needs to construct one client if they only care about one aspect of the client.

@DartBot
Copy link
Author

DartBot commented Jul 14, 2014

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


Hi, thanks for the explanation, that makes a lot of sense.

@nex3
Copy link
Member

nex3 commented Jul 17, 2014

Issue #10411 has been merged into this issue.

@DartBot
Copy link
Author

DartBot commented Jul 21, 2014

This comment was originally written by @seaneagan


Doesn't Future.timeout solve this pretty well already?

https://api.dartlang.org/apidocs/channels/stable/dartdoc-viewer/dart-async.Future#id_timeout

Example:

    http.read('http://google.com')
        .timeout(const Duration(seconds: 5))
        .then(...);

@kevmoo
Copy link
Member

kevmoo commented Jan 21, 2015

I don't think HttpTimeoutClient can accomplish what we want.

Users would like to actually cancel the inflight request.

dart:html has abort() on HttpRequest
dart:io HttpClient* has no equivalente – the idleTimeout just manages keep-alive connections while idea AKAICT


Added Area-Pkg label.

@kevmoo
Copy link
Member

kevmoo commented Feb 4, 2015

Marked this as being blocked by #22265.

@DartBot DartBot added Type-Enhancement area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. labels Feb 4, 2015
@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

This issue has been moved to dart-lang/http#21.

@DartBot DartBot closed this as completed Jun 5, 2015
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed triaged labels Mar 1, 2016
@michpolicht
Copy link

This comment was originally written by @seaneagan

Doesn't Future.timeout solve this pretty well already?

https://api.dartlang.org/apidocs/channels/stable/dartdoc-viewer/dart-async.Future#id_timeout

Example:

http.read('http://google.com')
    .timeout(const Duration(seconds: 5))
    .then(...);

What if HTTP request succeeds after 5 seconds?

@fzyzcjy
Copy link
Contributor

fzyzcjy commented Aug 1, 2021

@michpolicht +1 I am afraid this will waste resource! if the http request last for, say, 20s, then even if it "timeouts", the resource is still be used!

@edigun-man
Copy link

This comment was originally written by @seaneagan
Doesn't Future.timeout solve this pretty well already?
https://api.dartlang.org/apidocs/channels/stable/dartdoc-viewer/dart-async.Future#id_timeout
Example:

http.read('http://google.com')
    .timeout(const Duration(seconds: 5))
    .then(...);

What if HTTP request succeeds after 5 seconds?

How about set timeout to max 5-10 seconds? so when the data is not there, you could show some alert dialog by onTimeout callbak to ask user to refresh the page? i think 10 seconds is too much for regular user to wait tbh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants