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

Timeout support (connect and execution) #28

Closed
Beanow opened this Issue Feb 25, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@Beanow
Copy link

Beanow commented Feb 25, 2015

For comparison curl provides you with two options:

  • CURLOPT_CONNECTTIMEOUT the time it may take to establish the connection.
  • CURLOPT_TIMEOUT the time the total execution of the request is allowed to take.

Both of these I'm currently using to "probe" ~150 servers per day and test their connect and response time. Since this is about speed, I'm not interested in keeping the connection open for more than a couple of seconds, so I would like to tweak both these parameters.

The timeout branch seemed to be aimed at the total execution time equivalent.
And the connection timeout is available in stream_socket_client, currently a fixed value Connector.php:43.

@afk11

This comment has been minimized.

Copy link

afk11 commented Jun 29, 2015

I think @clue's last post in #12 may provide a means to achieve this neatly. Otherwise, we're looking at a slate of reimplementations of the same logic.

Issues of react end users regarding timeouts (there are bound to be more):
reactphp/http-client#28
clue/reactphp-redis#10
ratchetphp/Ratchet#316
reactphp-legacy/whois#3 (around since 2012)

@clue

This comment has been minimized.

Copy link
Contributor

clue commented Jul 1, 2015

Timeout support (connect and execution)

I'm all for adding support for connection timeout handling! 👍

However, I'm not sure how the "execution timeout" applies in the context of this library (rather low-level connection establishment). Can you explain what you're referring to with "execution" timeout (or "the time the total execution of the request is allowed to take").

Issues of react end users regarding timeouts (there are bound to be more) […]

Thanks for summing this up @afk11! Indeed, there are quite a few open tickets that deal with connection timeouts (and probably many more we're not even aware of).

For this reason, I've started working on a very simple low-level timeout library (https://github.com/clue/php-promise-timeout). This library can easily be embedded in order to add timeout support to pretty much any Promise-based API.

In this particular case, here's a prototype for how this could be used in this library (dedicated TimeoutConnector): https://gist.github.com/clue/75d00d9c2ca2e931661c#file-timeoutconnector-php-L17

Open issue before I feel comfortable filing a PR is where this library should live. IMO it should be part of the ReactPHP namespace, so here's a ticket for discussing this further: reactphp/promise-timer#2

Any input is welcome!

@clue

This comment has been minimized.

Copy link
Contributor

clue commented Nov 21, 2015

Any input is welcome!

This ticket has been open for quite some time already :) The timeout handling is now part of the new react/promise-timer component, see also PR #51 which integrates this here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment