Skip to content
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.

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

Handle Elastica Transport errors #1465

Merged
merged 11 commits into from Jan 29, 2019
2 changes: 1 addition & 1 deletion composer.json
Expand Up @@ -19,7 +19,7 @@
"symfony/property-access": "^3.4|^4",
"pagerfanta/pagerfanta": "^1.0.5|^2.0",
"psr/log": "^1.0",
"ruflin/elastica": "^5.3|^6.1"
"ruflin/elastica": "^5.3.5|^6.1.1"
},
"require-dev": {
"doctrine/orm": "^2.5",
Expand Down
9 changes: 9 additions & 0 deletions src/DependencyInjection/Configuration.php
Expand Up @@ -452,6 +452,15 @@ private function addClientsSection(ArrayNodeDefinition $rootNode)
->scalarNode('host')->end()
->scalarNode('port')->end()
->scalarNode('proxy')->end()
->arrayNode('http_error_codes')
->beforeNormalization()
->ifTrue(function ($v) { return !is_array($v); })
->then(function ($v) { return array($v); })
->end()
->cannotBeEmpty()
->defaultValue([400, 403, 404])
->prototype('scalar')->end()
->end()
->scalarNode('aws_access_key_id')->end()
->scalarNode('aws_secret_access_key')->end()
->scalarNode('aws_region')->end()
Expand Down
11 changes: 11 additions & 0 deletions src/Elastica/Client.php
Expand Up @@ -12,6 +12,7 @@
namespace FOS\ElasticaBundle\Elastica;

use Elastica\Client as BaseClient;
use Elastica\Exception\ClientException;
use Elastica\Request;
use FOS\ElasticaBundle\Logger\ElasticaLogger;
use Symfony\Component\Stopwatch\Stopwatch;
Expand Down Expand Up @@ -50,6 +51,16 @@ public function request($path, $method = Request::GET, $data = [], array $query
$response = parent::request($path, $method, $data, $query, $contentType);
$responseData = $response->getData();

$transportInfo = $response->getTransferInfo();
$connection = $this->getLastRequest()->getConnection();
$forbiddenHttpCodes = $connection->hasConfig('http_error_codes') ? $connection->getConfig('http_error_codes') : [];

if (isset($transportInfo['http_code']) && in_array($transportInfo['http_code'], $forbiddenHttpCodes, true)) {
$body = is_array($responseData) ? json_encode($responseData) : $responseData;
$message = sprintf('Error in transportInfo: response code is %s, response body is %s', $transportInfo['http_code'], $body);
throw new ClientException($message);
}

if (isset($responseData['took']) && isset($responseData['hits'])) {
$this->logQuery($path, $method, $data, $query, $response->getQueryTime(), $response->getEngineTime(), $responseData['hits']['total']);
} else {
Expand Down
24 changes: 24 additions & 0 deletions tests/Unit/DependencyInjection/ConfigurationTest.php
Expand Up @@ -310,6 +310,30 @@ public function testAWSConfig()
$this->assertTrue($connection['ssl']);
}

public function testHttpErrorCodesConfig()
{
// test defaults
$configuration = $this->getConfigs([
'clients' => [
'default' => [
],
],
]);
$connection = $configuration['clients']['default']['connections'][0];
$this->assertSame([400, 403, 404], $connection['http_error_codes']);

// test custom
$configuration = $this->getConfigs([
'clients' => [
'default' => [
'http_error_codes' => ['HTTP_ERROR_CODE']
],
],
]);
$connection = $configuration['clients']['default']['connections'][0];
$this->assertSame(['HTTP_ERROR_CODE'], $connection['http_error_codes']);
}

private function getConfigs(array $configArray)
{
$configuration = new Configuration(true);
Expand Down
71 changes: 61 additions & 10 deletions tests/Unit/Elastica/ClientTest.php
Expand Up @@ -11,24 +11,49 @@

namespace FOS\ElasticaBundle\Tests\Unit\Client;

use Elastica\Client as BaseClient;
use Elastica\Connection;
use Elastica\JSON;
use Elastica\Request;
use Elastica\Response;
use Elastica\Transport\NullTransport;
use FOS\ElasticaBundle\Elastica\Client;
use FOS\ElasticaBundle\Logger\ElasticaLogger;
use PHPUnit\Framework\TestCase;
use Elastica\Exception\ClientException;

class ClientTest extends TestCase
{
public function testRequestsAreLogged()
private function getConnectionMock()
{
$connection = $this->createMock(Connection::class);
$connection->expects($this->any())->method('toArray')->will($this->returnValue([]));
return $connection;
}

private function getClientMock(Response $response = null, $connection = null)
{
$transport = new NullTransport();
if ($response) {
$transport->setResponse($response);
}

$connection = $this->createMock(Connection::class);
if (!$connection) {
$connection = $this->getConnectionMock();
}
$connection->expects($this->any())->method('getTransportObject')->will($this->returnValue($transport));
$connection->expects($this->any())->method('toArray')->will($this->returnValue([]));

$client = $this->getMockBuilder(Client::class)
->setMethods(['getConnection'])
->getMock();

$client->expects($this->any())->method('getConnection')->will($this->returnValue($connection));
return $client;
}

public function testRequestsAreLogged()
{
$client = $this->getClientMock();
$logger = $this->createMock(ElasticaLogger::class);
$logger
->expects($this->once())
Expand All @@ -44,17 +69,43 @@ public function testRequestsAreLogged()
$this->isType('array'),
$this->isType('array')
);

$client = $this->getMockBuilder(Client::class)
->setMethods(['getConnection'])
->getMock();

$client->expects($this->any())->method('getConnection')->will($this->returnValue($connection));

$client->setLogger($logger);

$response = $client->request('foo');

$this->assertInstanceOf(Response::class, $response);
}

public function testRequestsWithTransportInfoErrorsRaiseExceptions()
{
$httpCode = 403;
$responseString = JSON::stringify(['message' => 'some AWS error']);
$transferInfo = [
'request_header' => 'bar',
'http_code' => $httpCode,
'body' => $responseString,
];
$response = new Response($responseString);
$response->setTransferInfo($transferInfo);

$connection = $this->getConnectionMock();
$connection
->expects($this->exactly(1))
->method('hasConfig')
->with('http_error_codes')
->willReturn(true)
;
$connection
->expects($this->exactly(1))
->method('getConfig')
->with('http_error_codes')
->willReturn([400, 403, 404])
;
$client = $this->getClientMock($response, $connection);

$desiredMessage = sprintf('Error in transportInfo: response code is %d, response body is %s', $httpCode, $responseString);
$this->expectException(ClientException::class);
$this->expectExceptionMessage($desiredMessage);
$response = $client->request('foo');
}
}