Got a lot of Connection timed out in my very broken test links. :-) Compared to wget I have not found any inconsistencies yet. So far so good.
If you believe this is a false error, turn off async_connect in the httprl options array and try again. Write.
This sounds like an impossible TODO for HTTPRL admins/users, isn't it? It's not a configuration option (I'm aware of) that users are able to change. Should we make this an optional setting in HTTPRL settings or a hidden settings.php setting or just remove the confusing text? Working around such issues is currently up to all maintainers using HTTPRL and no effected site can change this without patching modules.
I'm just asking for a usability discussion how we should allow users to change this on their own, but I would prefer to have a setting in HTTPRL as this setting sounds like a global setting that should affect all modules integrating with HTTPRL. I would also remove the confusing message users cannot recover from. Not sure why this message is there... maybe it's for the async PHP bug that we identified in 5.2.x ?
Comments
Comment #1
mikeytown2 CreditAttribution: mikeytown2 commentedIn the options array that is passed into httprl_request() set
'async_connect' => FALSE
and try again.This patch has been committed. It only displays the extra text if the socket name is empty and the async_connect option is set to TRUE. Let me know if you get the extra text now :)
If you do, try increasing the global connections limit as that will reduce the occurrences of this. Explanation of why increasing the global limit will help:
$empty_runs is a counter that gets incremented if stream_select returns empty. On success $empty_runs is reset to zero. stream_select() returns the number of stream resources contained in the modified arrays, which may be zero if the timeout expires before anything interesting happens. If this timeout has been hit after 25ms*32 ~ 800ms + 5ms*32 = 160ms Grand Total of about 1 second (at the very start of the loop the timeout can be 2 seconds); then HTTPRL checks to see if any of the sockets haven't made a connection yet VIA stream_socket_get_name. If this function returns FALSE that means a connection to the endpoint server has not been made yet; it's a way to see when a stream is ready if an asynchronous connection was made.
All of the above only happens if ALL streams return back in a not ready state. Thus the more streams that are in the connection pool, the chance of having all of them take a long time to complete a TCP connection is reduced. If all connections are dead/slow then it would be best to remove the dead/slow ones so other connections can be made and not timeout the function.
Comment #2
mikeytown2 CreditAttribution: mikeytown2 commentedAlso committed this
Comment #3
hass CreditAttribution: hass commentedPlease remove the trailing space from the translatable strings. Translators may not see them. Just write
. ' ' .
to glue both strings together, please. I give dev a new try later.However it was not about me, who is able o set the async option... It's our users who need to patch other maintainers modules just to try out... This sounds tooo difficult to me.
Comment #4
mikeytown2 CreditAttribution: mikeytown2 commentedCommitted this patch for the white space issue.
Comment #5
mikeytown2 CreditAttribution: mikeytown2 commentedFollowing patch has been committed.
Comment #6
mikeytown2 CreditAttribution: mikeytown2 commentedI think I need more than 1 timeout setting. wget has multiple timeouts
http://www.gnu.org/software/wget/manual/html_node/Download-Options.html#...
I've listed where the equivalent is in HTTPRL
When async_connect is TRUE
--dns-timeout is done in stream_socket_client()
--connect-timeout is done in httprl_send_request() via stream_socket_get_name()
--read-timeout is done in httprl_send_request() via microtime
When async_connect is FALSE
--dns-timeout is done in stream_socket_client()
--connect-timeout is done in stream_socket_client()
--read-timeout is done in httprl_send_request() via microtime
Would you be in favor of multiple timeout settings?
Comment #7
hass CreditAttribution: hass commentedUntil now I had no need for this yet...
Have you seen this with the 900 seconds read timeout? Sounds a bit high...
Comment #8
mikeytown2 CreditAttribution: mikeytown2 commenteddns-timeout + connect-timeout. PHP default is about 20 seconds.
read-timeout. Drupal uses 30 seconds from the start of the function call which is effectively (dns-timeout + connect-timeout) - 30 = How much time is left for the read-timeout.
If HTTPRL had different timeouts for each of these I would feel confident in removing the async_connect message, as one could adjust the connect-timeout setting. I would make the defaults for these timeouts user configurable then.
Proposed Error codes for said timeouts:
dns-timeout 11001 "Host not found. No such host is known." It's the best match I could find.
connect-timeout 10060 "Connection timed out. A connection attempt failed because the connected party did not properly respond after a period of time, or the established connection failed because the connected host has failed to respond."
read-timeout 10053 "Software caused connection abort. An established connection was aborted by the software in your host machine, possibly due to a data transmission time-out or protocol error. "
Comment #9
hass CreditAttribution: hass commentedSounds all good to me.
Comment #10
mikeytown2 CreditAttribution: mikeytown2 commentedProposed defaults
5 seconds for DNS
5 seconds for connect
20 seconds for first byte
30 seconds for the whole thing (what the current timeout setting controls)
All 4 timeouts will be configurable from GUI.
Comment #11
mikeytown2 CreditAttribution: mikeytown2 commentedFollowing patch has not been committed. GUI options are not in yet. Minimal testing done.
New thing added is to the object is time_to_first_byte.
Comment #12
mikeytown2 CreditAttribution: mikeytown2 commentedThis patch has been committed to 6.x & 7.x.
Also included in this patch are fixes for
#1412474: Missing response codes in drupal_http_request
#1414368: Drupal_http_request does not handle basic auth correctly when dealing with blank passwords
Comment #13
mikeytown2 CreditAttribution: mikeytown2 commentedAttached patch has the displayed default values come from the defined constants. It has been applied to 6.x & 7.x
Comment #14
hass CreditAttribution: hass commentedJust for the usability I would name the fields like "DNS timeout (
dns_timeout
)" and move the "Maximum number of seconds a DNS lookup request may take." to the description. Additionally I would end the sentence with a period. I would also use'#suffix' = t('seconds')
:-)How about using http://api.drupal.org/api/drupal/includes%21common.inc/function/format_i... for formatting the seconds in t()?
Had some time to do a test. Looks great here. Found ~40 Connection timed out. TCP Connect Timeout. and one Connection timed out. Time to First Byte Timeout. with
http://www.nonprofit.de
.http://www.nonprofit.de
seems to be an extremely slow site, but gives me a200 OK
with wget.Comment #15
hass CreditAttribution: hass commentedUhhh... shouldn't the max() value win?
Comment #16
hass CreditAttribution: hass commented#15 is not clear to me. How should a user override a wrong value? I think it should be max() between this setting and the setting a maintainer may have set in options array (or what the defaults are).
Patch attached fixes code style issues and usability issues. I left the interval formatting out.
Comment #17
hass CreditAttribution: hass commentedMissed a closing bracket
Comment #18
mikeytown2 CreditAttribution: mikeytown2 commentedCommitted #17. Thanks for the patch :)
#16
The defaults get added into the options array via +. Same thing as overriding GET (the default) with HEAD. In terms of max(), that comes into play with the global timeout and connection (global & per domain) limits. All other timeouts are per connection. So in link checkers case, it can run for at least 180 seconds because the global timeout has been set to 180 and 120 is less than that.
What we could do in link checker is
Comment #19
mikeytown2 CreditAttribution: mikeytown2 commentedAttached is a patch that will adjust the TTFB timeout if timeout is larger than the default and ttfb is still the default. It has been committed to 6.x & 7.x
Comment #20.0
(not verified) CreditAttribution: commentedUpdated issue summary.