Conflict between timeout option of Connection::ping and ClientBuilder::connectionParams

[JMLamodiere]

Summary of problem or feature request

TL;DR

To merge a multi-dimensionnal array with defaults defined in another, as done in Connection::performRequest, we should use array-replace-recursive instead of array-merge-recursive. Here is a runnable snippet

The bug

There is a bug when merging timeout param with a connection pool other than the default one (StaticNoPingConnectionPool) and when a global timeout is defined at connection level.

People affected

Those using another ConnectionPool that the default one (StaticNoPingConnectionPool) AND defining a timeout at connection level.

Description

PR #507 has added the ability to set defaults params at connection level instead of per request, such as $params['client']['timeout']

The method Connection::ping() will also set a client timeout to 1. This method ping is not called when using the default connection pool (StaticNoPingConnectionPool), but is used with others, such as StaticConnectionPool .

When timeout is set in those 2 places, the method Connection::performRequest will merge them with array_merge_recursive. But this php function as a weird behaviour : instead of overriding this integer param, it replace it with an array containing the 2 values.

It will result in a bug when GuzzleHttp\Ring\Client\CurlFactory::applyHandlerOptions tries to multiply this value *1000 : the value contains an array instead of an integer and cannot be multiplied.

We should use array_replace_recursive instead

Code snippet of problem

Here is a runnable snippet

$builder = new ClientBuilder();
$builder->setConnectionParams(['client' => ['timeout' => 5]]);
$builder->setConnectionPool('Elasticsearch\ConnectionPool\StaticConnectionPool');

At the first request :

Fatal error: Unsupported operand types in [...]/vendor/guzzlehttp/ringphp/src/Client/CurlFactory.php on line 424

Call Stack:
    2.3379   57060368  10. Elasticsearch\Client->bulk(array(1)) [...]
    2.3598   58102368  11. Elasticsearch\Endpoints\AbstractEndpoint->performRequest() [...]/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Client.php:864
    2.3598   58102912  12. Elasticsearch\Transport->performRequest(string(4), string(11), array(0), string(953821), array(0)) [...]/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Endpoints/AbstractEndpoint.php:80
    2.3598   58102960  13. Elasticsearch\Transport->getConnection() [...]/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Transport.php:89
    2.3598   58103152  14. Elasticsearch\ConnectionPool\StaticConnectionPool->nextConnection(???) [...]/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Transport.php:71
    2.3599   58104328  15. Elasticsearch\Connections\Connection->ping() [...]/vendor/elasticsearch/elasticsearch/src/Elasticsearch/ConnectionPool/StaticConnectionPool.php:60
    2.3599   58105600  16. Elasticsearch\Connections\Connection->performRequest(string(4), string(1), null, null, array(1), ???) [...]/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php:417
    2.3599   58108208  17. Elasticsearch\Connections\Connection->Elasticsearch\Connections\{closure}(array(6), class Elasticsearch\Connections\Connection, null, array(1)) [...]/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php:159
    2.3605   58175616  18. GuzzleHttp\Ring\Client\Middleware::GuzzleHttp\Ring\Client\{closure}(array(6)) [...]/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php:184
    2.3605   58175656  19. GuzzleHttp\Ring\Client\CurlHandler->__invoke(array(6)) [...]/vendor/guzzlehttp/ringphp/src/Client/Middleware.php:30
    2.3614   58211320  20. GuzzleHttp\Ring\Client\CurlHandler->_invokeAsArray(array(6)) [...]/vendor/guzzlehttp/ringphp/src/Client/CurlHandler.php:68
    2.3614   58213296  21. GuzzleHttp\Ring\Client\CurlFactory->__invoke(array(6), resource(995) of type (curl)) [...]/vendor/guzzlehttp/ringphp/src/Client/CurlHandler.php:84
    2.3615   58216528  22. GuzzleHttp\Ring\Client\CurlFactory->applyHandlerOptions(array(6), array(9)) [...]/vendor/guzzlehttp/ringphp/src/Client/CurlFactory.php:33

The $value contains array (0 => 5, 1 => 1) (both timeouts) whereas it should contain an integer (one of the timeouts).

System details

• Operating System : Ubuntu 16.04
• PHP Version : 7.0.15 for 5.x branch, 5.6.25 for 2.x branch
• ES-PHP client version : same
• Elasticsearch version : 5.3 for 5.x branch, 2.4 for 2.x branch

[polyfractal]

Oh, PHP. :(

Thanks for this! Kinda amazed it's taken me/the client this long to stumble into this particular bug

[polyfractal]

I cherry-picked to 5.0 branch as well :)