diff --git a/composer.json b/composer.json index 04c5d57..f0e8419 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,7 @@ "monolog/monolog": "^3.0", "phpunit/phpunit": "^10.5", "psr/log": "^3.0", - "tarantool/phpunit-extras": "^0.3.0", + "tarantool/phpunit-extras": "^0.3.2", "vimeo/psalm": "^5.23|^6" }, "suggest": { diff --git a/src/Exception/RequestFailed.php b/src/Exception/RequestFailed.php index a151962..5e4fe3f 100644 --- a/src/Exception/RequestFailed.php +++ b/src/Exception/RequestFailed.php @@ -28,15 +28,28 @@ public function getError() : ?Error public static function fromErrorResponse(Response $response) : self { - $self = new self( - $response->getBodyField(Keys::ERROR_24), - $response->getCode() & (Response::TYPE_ERROR - 1) - ); + $error = null; + if ($errorMap = $response->tryGetBodyField(Keys::ERROR)) { + try { + $error = Error::fromMap($errorMap); + } catch (\InvalidArgumentException|\TypeError $exception) { + throw new UnexpectedResponse(\sprintf('Invalid response body field 0x%x', Keys::ERROR), 0, $exception); + } + } + + $message = $response->tryGetBodyField(Keys::ERROR_24); + if ($response->hasBodyField(Keys::ERROR_24) && !\is_string($message)) { + throw new UnexpectedResponse(\sprintf('Invalid response body field 0x%x', Keys::ERROR_24)); + } - if ($error = $response->tryGetBodyField(Keys::ERROR)) { - $self->error = Error::fromMap($error); + $message ??= $error?->getMessage(); + if (null === $message) { + throw new UnexpectedResponse(\sprintf('Missing response body field 0x%x', Keys::ERROR_24)); } + $self = new self($message, $response->getCode() & (Response::TYPE_ERROR - 1)); + $self->error = $error; + return $self; } diff --git a/src/Response.php b/src/Response.php index d16a28d..fe1064e 100644 --- a/src/Response.php +++ b/src/Response.php @@ -13,6 +13,8 @@ namespace Tarantool\Client; +use Tarantool\Client\Exception\UnexpectedResponse; + final class Response { public const TYPE_ERROR = 0x8000; @@ -22,15 +24,17 @@ final class Response public function __construct(array $header, array $body) { + self::validateHeaderField($header, Keys::CODE); + self::validateHeaderField($header, Keys::SYNC); + self::validateHeaderField($header, Keys::SCHEMA_ID); + $this->header = $header; $this->body = $body; } public function isError() : bool { - $code = $this->header[Keys::CODE]; - - return $code >= self::TYPE_ERROR; + return $this->getCode() >= self::TYPE_ERROR; } public function getCode() : int @@ -54,7 +58,7 @@ public function getBodyField(int $key) return $this->body[$key]; } - throw new \OutOfRangeException(\sprintf('The body key 0x%x does not exist', $key)); + throw new UnexpectedResponse(\sprintf('Missing response body field 0x%x', $key)); } public function tryGetBodyField(int $key, $default = null) @@ -66,4 +70,11 @@ public function hasBodyField(int $key) : bool { return \array_key_exists($key, $this->body); } + + private static function validateHeaderField(array $header, int $key) : void + { + if (!\is_int($header[$key] ?? null)) { + throw new UnexpectedResponse(\sprintf('Missing or invalid response header field 0x%x', $key)); + } + } } diff --git a/tests/Unit/Exception/RequestFailedTest.php b/tests/Unit/Exception/RequestFailedTest.php new file mode 100644 index 0000000..8fdbf2d --- /dev/null +++ b/tests/Unit/Exception/RequestFailedTest.php @@ -0,0 +1,70 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Tarantool\Client\Tests\Unit\Exception; + +use PHPUnit\Framework\TestCase; +use Tarantool\Client\Error; +use Tarantool\Client\Exception\RequestFailed; +use Tarantool\Client\Exception\UnexpectedResponse; +use Tarantool\Client\Keys; +use Tarantool\Client\Response; + +final class RequestFailedTest extends TestCase +{ + public function testUsesStructuredErrorWhenLegacyMessageIsMissing() : void + { + $error = new Error('ClientError', 'file.lua', 1, 'The server error', 1, 42); + $response = new Response( + [Keys::CODE => Response::TYPE_ERROR + 42, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], + [Keys::ERROR => $error->toMap()] + ); + + $requestFailed = RequestFailed::fromErrorResponse($response); + $actualError = $requestFailed->getError(); + + self::assertSame('The server error', $requestFailed->getMessage()); + self::assertNotNull($actualError); + self::assertSame($error->getMessage(), $actualError->getMessage()); + } + + public function testMissingErrorMessageWithoutStructuredErrorIsUnexpectedResponse() : void + { + $response = new Response([Keys::CODE => Response::TYPE_ERROR, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], []); + + $this->expectException(UnexpectedResponse::class); + RequestFailed::fromErrorResponse($response); + } + + public function testRejectsNonArrayStructuredError() : void + { + $response = new Response( + [Keys::CODE => Response::TYPE_ERROR, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], + [Keys::ERROR => 'malformed', Keys::ERROR_24 => 'The server error'] + ); + + $this->expectException(UnexpectedResponse::class); + RequestFailed::fromErrorResponse($response); + } + + public function testTranslatesMalformedStructuredErrorMap() : void + { + $response = new Response( + [Keys::CODE => Response::TYPE_ERROR, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], + [Keys::ERROR => [Keys::ERROR_STACK => []], Keys::ERROR_24 => 'The server error'] + ); + + $this->expectException(UnexpectedResponse::class); + RequestFailed::fromErrorResponse($response); + } +} diff --git a/tests/Unit/ResponseTest.php b/tests/Unit/ResponseTest.php index 405baab..2d0ad48 100644 --- a/tests/Unit/ResponseTest.php +++ b/tests/Unit/ResponseTest.php @@ -14,6 +14,7 @@ namespace Tarantool\Client\Tests\Unit; use PHPUnit\Framework\TestCase; +use Tarantool\Client\Exception\UnexpectedResponse; use Tarantool\Client\Keys; use Tarantool\Client\Response; @@ -22,21 +23,59 @@ final class ResponseTest extends TestCase public function testGetSchemaIdReturnsCorrectId() : void { $schemaId = 42; - $response = new Response([Keys::SCHEMA_ID => $schemaId], []); + $response = new Response([Keys::CODE => 0, Keys::SYNC => 1, Keys::SCHEMA_ID => $schemaId], []); self::assertSame($schemaId, $response->getSchemaId()); } + public function testConstructorRejectsMissingCode() : void + { + $this->expectException(UnexpectedResponse::class); + new Response([Keys::SYNC => 1], []); + } + + public function testConstructorRejectsInvalidCodeType() : void + { + $this->expectException(UnexpectedResponse::class); + new Response([Keys::CODE => '0', Keys::SYNC => 1], []); + } + + public function testConstructorRejectsMissingSync() : void + { + $this->expectException(UnexpectedResponse::class); + new Response([Keys::CODE => 0], []); + } + + public function testConstructorRejectsInvalidSchemaIdType() : void + { + $this->expectException(UnexpectedResponse::class); + new Response([Keys::CODE => 0, Keys::SYNC => 1, Keys::SCHEMA_ID => null], []); + } + + public function testConstructorRejectsMissingSchemaId() : void + { + $this->expectException(UnexpectedResponse::class); + new Response([Keys::CODE => 0, Keys::SYNC => 1], []); + } + + public function testGetBodyFieldRejectsMissingFieldAsUnexpectedResponse() : void + { + $response = new Response([Keys::CODE => 0, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], []); + + $this->expectException(UnexpectedResponse::class); + $response->getBodyField(Keys::DATA); + } + public function testHasBodyKeyReturnsTrue() : void { - $response = new Response([], [42 => null]); + $response = new Response([Keys::CODE => 0, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], [42 => null]); self::assertTrue($response->hasBodyField(42)); } public function testHasBodyKeyReturnsFalse() : void { - $response = new Response([], [24 => null]); + $response = new Response([Keys::CODE => 0, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], [24 => null]); self::assertFalse($response->hasBodyField(42)); }