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

In-built request timeout functionality #28

Closed
Renegade334 opened this issue May 10, 2015 · 6 comments
Closed

In-built request timeout functionality #28

Renegade334 opened this issue May 10, 2015 · 6 comments

Comments

@Renegade334
Copy link

Currently, request timeouts can be implemented somewhat messily by specifying loop timers and attempting to manually close the request. They should ideally be in-built.

@cboden
Copy link
Member

cboden commented May 10, 2015

This feature has been started in the underlying library reactphp-legacy/socket-client#10. Once it's implemented there it should be simple enough to add here.

@cboden cboden added the blocked label May 10, 2015
@WyriHaximus WyriHaximus self-assigned this May 10, 2015
@clue
Copy link
Member

clue commented Jun 6, 2015

👍 on supporting a timeout setting.

Though I'm not sure what the API could look like…
Also, when should we actually time out (connection timeout, whole request timeout, idle timeout etc…)?

I think it makes sense to add an API similar to reactphp-legacy/socket-client#10. Any thoughts on this @Renegade334?

@Renegade334
Copy link
Author

Clients like cURL support both a connection timeout and a total timeout. Both would be useful, IMO, although I was originally thinking of total timeout when submitting this. The latter would be a client-level feature, I guess, whereas the former would be socket-level.

renegade@mercury:~$ curl --connect-timeout 1 http://10.255.255.1/
curl: (28) Connection timed out after 1001 milliseconds
renegade@mercury:~$ netcat -l 3456 &> /dev/null & curl --max-time 1 http://localhost:3456/
curl: (28) Operation timed out after 1000 milliseconds with 0 bytes received

@WyriHaximus
Copy link
Member

@Renegade334 yes that makes sense. We could even go for 3 time outs:

  • Connect timeout, how long we wait for the connection to be created, done in socket-client
  • Response timeout, how long a server can take before responding
  • Response body timeout, how long it can take before the entire response body is downloaded

@clue
Copy link
Member

clue commented Sep 3, 2015

FWIW: This is likely related to #36. Possible implementation approach could be to just close() the request once the timeout timer fires.

@clue
Copy link
Member

clue commented Jul 13, 2020

I'm closing this as it is already supported by the new HTTP client that has been merged into react/http via reactphp/http#368 :shipit: You should upgrade as per #152 👍

@clue clue closed this as completed Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants