Problem/Motivation

Simpletest doesn't have any timeout configured, so the default is chosen:

CURLOPT_FTP_RESPONSE_TIMEOUT: Indefinite
CURLOPT_TIMEOUT: Indefinite
CURLOPT_TIMEOUT_MS: Indefinite
CURLOPT_CONNECTTIMEOUT: 300 seconds
CURLOPT_CONNECTTIMEOUT_MS: Indefinite
CURLOPT_ACCEPTTIMEOUT_MS: 60 seconds

(source: https://stackoverflow.com/questions/10308915/php-default-curl-timeout-value)

In BTB we use the default guzzle configuration which is using, the default from Drupal: \Drupal\Core\Http\ClientFactory::fromOptions, which is 30 seconds.

While this is not a problem in normal test running, it is a problem when you debug the site using xdebug.

Proposed resolution

Set the default value to infinite in BTB

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#18 remove_curl_timeout_in-2784159-18.patch1.01 KBjibran
#12 2784159-12.patch823 bytesalexpott
#5 2784159-5.patch819 byteshchonov
#2 2784159-2.patch605 bytesdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner created an issue. See original summary.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Active » Needs review
FileSize
2784159-2.patch605 bytes

Here is some fix for that

Status: Needs review » Needs work

The last submitted patch, 2: 2784159-2.patch, failed testing.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented 
+++ b/core/tests/Drupal/Tests/BrowserTestBase.php
@@ -336,7 +336,7 @@ protected function initMink() {
-      $driver->getClient()->setClient(\Drupal::httpClient());
+      $driver->getClient()->setClient(\Drupal::service('http_client_factory')->fromOptions(['timeout' => NULL]);

If we rewrite the line we should not use \Drupal but $this->container instead.

And we should add a comment why we set the timeout to NULL.

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
Status: Needs work » Needs review
FileSize
2784159-5.patch819 bytes

Re-roll + addressing #4.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Needs review » Reviewed & tested by the community

Great, thank you @hchonov!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

So $this->container is absolutely no different to using \Drupal especially at this point in the test infra. And actually later on \Drupal access is better than $this->container because $this->container gets out-of-date in functional and functional javascript tests. Ideally we wouldn't be using the run-time container at all and test runners would have their own container that inject their own dependencies. But that is all for another issue - just pointing out the the change asked for in #4 is at best moot and in the long term is something we need to refactor either way.

Committed c1a79b2 and pushed to 8.3.x. Thanks!

It didn't apply to 8.2.x otherwise I would have backported it since we should be able to debug that branch simply too.

  • alexpott committed 716fe71 on 8.3.x 
    Issue #2784159 by dawehner, hchonov: Remove CURL timeout in BTB
mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio commented

Could this be causing the root cause 0 byte problem in #2828559: UpdatePathTestBase tests randomly failing?

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented

The problem existed in the referenced issue even before this one got in.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Version: 8.3.x-dev » 8.2.x-dev
Status: Closed (fixed) » Reviewed & tested by the community
FileSize
2784159-12.patch823 bytes
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

#12 was posted by @alexpott for fixing #2844509: Random fails in BrowserTestBase tests with Operation timed out after 30001 milliseconds with 0 bytes.

  • catch committed e4b5647 on 8.2.x 
    Issue #2784159 by dawehner, alexpott, hchonov: Remove CURL timeout in...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.2.x, thanks!

  • alexpott committed 716fe71 on 8.4.x 
    Issue #2784159 by dawehner, hchonov: Remove CURL timeout in BTB

  • alexpott committed 716fe71 on 8.4.x 
    Issue #2784159 by dawehner, hchonov: Remove CURL timeout in BTB
jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
FileSize
remove_curl_timeout_in-2784159-18.patch1.01 KB

Core patch for 8.2.5. Please ignore the comment and patch.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.