elastic / elasticsearch-php

Official PHP client for Elasticsearch.
https://www.elastic.co/guide/en/elasticsearch/client/php-api/current/index.html
MIT License
5.25k stars 963 forks source link

ServerError was not handled correctly. #1387

Closed wgevaert closed 5 months ago

wgevaert commented 6 months ago

I just want to report this error I received. It was on version 6.8, but it seems versions 7.x are affected too, judging from the code base.

Summary of problem or feature request

I received the following error while running php mediawiki/core/maintenance/update.php with SMW installed.

Error from line 703 of /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php: Wrong
 parameters for Elasticsearch\\Common\\Exceptions\\ServerErrorResponseException([string $message [, long $code [, Throwable $previous = NULL]]])
Backtrace:
#0 /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php(703): Exception->__construct()
#1 /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php(315): Elasticsearch\\Connections\\Connection->process5xxError()
#2 /var/www/html/vendor/react/promise/src/FulfilledPromise.php(28): Elasticsearch\\Connections\\Connection->Elasticsearch\\Connections\\{closure}()
#3 /var/www/html/vendor/ezimuel/ringphp/src/Future/CompletedFutureValue.php(65): React\\Promise\\FulfilledPromise->then()
#4 /var/www/html/vendor/ezimuel/ringphp/src/Core.php(341): GuzzleHttp\\Ring\\Future\\CompletedFutureValue->then()
#5 /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php(332): GuzzleHttp\\Ring\\Core::proxy()
#6 /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php(204): Elasticsearch\\Connections\\Connection->Elasticsearch\\Con
nections\\{closure}()
#7 /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Transport.php(116): Elasticsearch\\Connections\\Connection->performRequest()
#8 /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Namespaces/BooleanRequestWrapper.php(49): Elasticsearch\\Transport->performRequest()
#9 /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Namespaces/IndicesNamespace.php(218): Elasticsearch\\Namespaces\\BooleanRequestWrapper::per
formRequest()
<big backtrace through 3rd party code>

Code snippet of problem

In vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php:

    function wrapHandler(...) {
        [...]
                    if (isset($response['body']) === true) {
                        $response['body'] = stream_get_contents($response['body']);
                        $this->lastRequest['response']['body'] = $response['body'];
                    }
                    [...]
                        $this->process5xxError($request, $response, $ignore);
        [...]
    }

    function process5xxError($request, $response, $ignore)
    {
        $statusCode = $response['status'];
        $responseBody = $response['body'];
        [...]
        if ($statusCode === 500 && strpos($responseBody, "RoutingMissingException") !== false) {
            $exception = new RoutingMissingException($exception->getMessage(), $statusCode, $exception);
        } elseif ($statusCode === 500 && preg_match('/ActionRequestValidationException.+ no documents to get/', $responseBody) === 1) {
            $exception = new NoDocumentsToGetException($exception->getMessage(), $statusCode, $exception);
        } elseif ($statusCode === 500 && strpos($responseBody, 'NoShardAvailableActionException') !== false) {
            $exception = new NoShardAvailableException($exception->getMessage(), $statusCode, $exception);
        } else {
            $exception = new ServerErrorResponseException($responseBody, $statusCode);
        }

I suspect that stream_get_contents($response['body']) returned false, and then the responseBody was propagated to new ServerErrorResponseException(false, $statusCode); which throws an error. Even when the stream read is not working, such an error should be handled more gracefully.

System details

ezimuel commented 5 months ago

I'm sorry for the late replay. We are not supporting anymore 6.x version of the client.

wgevaert commented 5 months ago

And what about 7.x version?

but it seems versions 7.x are affected too, judging from the code base.