Skip to content

Fix #977: Handle the exception as structured even if the reason is null #980

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Dec 19, 2019
Merged

Fix #977: Handle the exception as structured even if the reason is null #980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
177bd95
Fix #977: Handle the exception as structured even if the reason is null
oogFranz Dec 3, 2019
84fedf0
Fix #977: Fix phpcs error
oogFranz Dec 3, 2019
e215a0b
Added unit test to #977 fix + defensive approach
ezimuel Dec 19, 2019
f148afa
Fixed CS
ezimuel Dec 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions src/Elasticsearch/Connections/Connection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,8 +679,15 @@ private function tryDeserializeError(array $response, string $errorClass): Elast
{
$error = $this->serializer->deserialize($response['body'], $response['transfer_stats']);
if (is_array($error) === true) {
if (isset($error['error']) === false) {
// <2.0 "i just blew up" nonstructured exception
// $error is an array but we don't know the format, reuse the response body instead
// added json_encode to convert into a string
return new $errorClass(json_encode($response['body']), (int) $response['status']);
}

// 2.0 structured exceptions
if (isset($error['error']['reason']) === true) {
if (is_array($error['error']) && array_key_exists('reason', $error['error']) === true) {
// Try to use root cause first (only grabs the first root cause)
$root = $error['error']['root_cause'];
if (isset($root) && isset($root[0])) {
Expand All @@ -694,18 +701,16 @@ private function tryDeserializeError(array $response, string $errorClass): Elast
$original = new $errorClass(json_encode($response['body']), $response['status']);

return new $errorClass("$type: $cause", (int) $response['status'], $original);
} elseif (isset($error['error']) === true) {
// <2.0 semi-structured exceptions
// added json_encode to convert into a string
$original = new $errorClass(json_encode($response['body']), $response['status']);

return new $errorClass($error['error'], (int) $response['status'], $original);
}

// <2.0 "i just blew up" nonstructured exception
// $error is an array but we don't know the format, reuse the response body instead
// <2.0 semi-structured exceptions
// added json_encode to convert into a string
return new $errorClass(json_encode($response['body']), (int) $response['status']);
$original = new $errorClass(json_encode($response['body']), $response['status']);

$errorEncoded = $error['error'];
if (is_array($errorEncoded)) {
$errorEncoded = json_encode($errorEncoded);
}
return new $errorClass($errorEncoded, (int) $response['status'], $original);
}

// if responseBody is not string, we convert it so it can be used as Exception message
Expand Down
35 changes: 35 additions & 0 deletions tests/Elasticsearch/Tests/Connections/ConnectionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@

use Elasticsearch\Client;
use Elasticsearch\ClientBuilder;
use Elasticsearch\Common\Exceptions\ServerErrorResponseException;
use Elasticsearch\Connections\Connection;
use Elasticsearch\Serializers\SerializerInterface;
use Elasticsearch\Serializers\SmartSerializer;
use Psr\Log\LoggerInterface;
use ReflectionClass;

class ConnectionTest extends \PHPUnit\Framework\TestCase
{
Expand Down Expand Up @@ -282,4 +285,36 @@ public function testGetHeadersContainBasicAuthOverHostArrayConfig()
$this->assertArrayNotHasKey('Authorization', $request['headers']);
$this->assertContains('username:password', $request['client']['curl'][CURLOPT_USERPWD]);
}

public function testTryDeserializeErrorWithMasterNotDiscoveredException()
{
$host = [
'host' => 'localhost'
];

$connection = new Connection(
function () {
},
$host,
[],
new SmartSerializer(),
$this->logger,
$this->trace
);

$reflection = new ReflectionClass(Connection::class);
$tryDeserializeError = $reflection->getMethod('tryDeserializeError');
$tryDeserializeError->setAccessible(true);

$body = '{"error":{"root_cause":[{"type":"master_not_discovered_exception","reason":null}],"type":"master_not_discovered_exception","reason":null},"status":503}';
$response = [
'transfer_stats' => [],
'status' => 503,
'body' => $body
];

$result = $tryDeserializeError->invoke($connection, $response, ServerErrorResponseException::class);
$this->assertInstanceOf(ServerErrorResponseException::class, $result);
$this->assertContains('master_not_discovered_exception', $result->getMessage());
}
}