diff --git a/src/Api/ErrorParser/AbstractErrorParser.php b/src/Api/ErrorParser/AbstractErrorParser.php index c72f03f647..0e96406b12 100644 --- a/src/Api/ErrorParser/AbstractErrorParser.php +++ b/src/Api/ErrorParser/AbstractErrorParser.php @@ -7,6 +7,7 @@ use Aws\Api\StructureShape; use Aws\CommandInterface; use Psr\Http\Message\ResponseInterface; +use SimpleXMLElement; abstract class AbstractErrorParser { @@ -27,23 +28,10 @@ public function __construct(?Service $api = null) } abstract protected function payload( - ResponseInterface $response, + ResponseInterface|SimpleXMLElement|array $responseOrParsedBody, StructureShape $member ); - protected function extractPayload( - StructureShape $member, - ResponseInterface $response - ) { - if ($member instanceof StructureShape) { - // Structure members parse top-level data into a specific key. - return $this->payload($response, $member); - } else { - // Streaming data is just the stream from the response body. - return $response->getBody(); - } - } - protected function populateShape( array &$data, ResponseInterface $response, @@ -57,16 +45,15 @@ protected function populateShape( if (!empty($data['code'])) { $errors = $this->api->getOperation($command->getName())->getErrors(); - foreach ($errors as $key => $error) { + foreach ($errors as $error) { // If error code matches a known error shape, populate the body if ($this->errorCodeMatches($data, $error)) { - $modeledError = $error; - $data['body'] = $this->extractPayload( - $modeledError, - $response + $data['body'] = $this->payload( + $data['parsed'] ?? $response, + $error, ); - $data['error_shape'] = $modeledError; + $data['error_shape'] = $error; foreach ($error->getMembers() as $name => $member) { switch ($member['location']) { diff --git a/src/Api/ErrorParser/JsonParserTrait.php b/src/Api/ErrorParser/JsonParserTrait.php index 67afb1645a..1a6e599960 100644 --- a/src/Api/ErrorParser/JsonParserTrait.php +++ b/src/Api/ErrorParser/JsonParserTrait.php @@ -3,7 +3,9 @@ use Aws\Api\Parser\PayloadParserTrait; use Aws\Api\StructureShape; +use GuzzleHttp\Psr7\Utils; use Psr\Http\Message\ResponseInterface; +use SimpleXMLElement; /** * Provides basic JSON error parsing functionality. @@ -37,10 +39,24 @@ private function genericHandler(ResponseInterface $response): array ); } - $parsedBody = null; $body = $response->getBody(); - if (!$body->isSeekable() || $body->getSize()) { - $parsedBody = $this->parseJson((string) $body, $response); + // If the body is not seekable then, read the full message + // before de-serializing + if (!$body->isSeekable()) { + $tempBody = Utils::streamFor(); + Utils::copyToStream($body, $tempBody); + $body = $tempBody; + } + + // Cast it to string + $body = (string) $body; + $parsedBody = []; + + // Avoid parsing an empty body + if (!empty($body)) { + // Parsing the body to avoid having to read the response body again. + // This will avoid issues when the body is not seekable + $parsedBody = $this->parseJson($body, $response); } // Parse error code from response body @@ -129,14 +145,13 @@ private function extractErrorCode(string $rawErrorCode): string } protected function payload( - ResponseInterface $response, + ResponseInterface|SimpleXMLElement|array $responseOrParsedBody, StructureShape $member ) { - $body = $response->getBody(); - if (!$body->isSeekable() || $body->getSize()) { - $jsonBody = $this->parseJson($body, $response); - } else { - $jsonBody = (string) $body; + $jsonBody = $responseOrParsedBody; + if ($responseOrParsedBody instanceof ResponseInterface) { + $body = $responseOrParsedBody->getBody(); + $jsonBody = $this->parseJson($body, $responseOrParsedBody); } return $this->parser->parse($member, $jsonBody); diff --git a/src/Api/ErrorParser/JsonRpcErrorParser.php b/src/Api/ErrorParser/JsonRpcErrorParser.php index 35e8ebe0ed..77da4087fe 100644 --- a/src/Api/ErrorParser/JsonRpcErrorParser.php +++ b/src/Api/ErrorParser/JsonRpcErrorParser.php @@ -28,20 +28,25 @@ public function __invoke( $data = $this->genericHandler($response); // Make the casing consistent across services. - if ($data['parsed']) { - $data['parsed'] = array_change_key_case($data['parsed']); + $parsed = $data['parsed'] ?? null; + if ($parsed) { + $parsed = array_change_key_case($data['parsed']); } - if (isset($data['parsed']['__type'])) { + if (isset($parsed['__type'])) { if (!isset($data['code'])) { - $parts = explode('#', $data['parsed']['__type']); - $data['code'] = isset($parts[1]) ? $parts[1] : $parts[0]; + $parts = explode('#', $parsed['__type']); + $data['code'] = $parts[1] ?? $parts[0]; } - $data['message'] = $data['parsed']['message'] ?? null; + + $data['message'] = $parsed['message'] ?? null; } $this->populateShape($data, $response, $command); + // Now lets make parsed to be all lowercase + $data['parsed'] = $parsed; + return $data; } } diff --git a/src/Api/ErrorParser/RestJsonErrorParser.php b/src/Api/ErrorParser/RestJsonErrorParser.php index 3d50a73ca6..259f132009 100644 --- a/src/Api/ErrorParser/RestJsonErrorParser.php +++ b/src/Api/ErrorParser/RestJsonErrorParser.php @@ -38,9 +38,14 @@ public function __invoke( $data['type'] = strtolower($data['type']); } + // Make the casing consistent across services. + $parsed = $data['parsed'] ?? null; + if ($parsed) { + $parsed = array_change_key_case($data['parsed']); + } + // Retrieve error message directly - $data['message'] = $data['parsed']['message'] - ?? ($data['parsed']['Message'] ?? null); + $data['message'] = $parsed['message'] ?? null; $this->populateShape($data, $response, $command); diff --git a/src/Api/ErrorParser/XmlErrorParser.php b/src/Api/ErrorParser/XmlErrorParser.php index 86f5d0be54..2378c66f4a 100644 --- a/src/Api/ErrorParser/XmlErrorParser.php +++ b/src/Api/ErrorParser/XmlErrorParser.php @@ -1,11 +1,13 @@ getBody(); - if ($body->getSize() > 0) { + if (!$body->isSeekable()) { + $tempBody = Utils::streamFor(); + Utils::copyToStream($body, $tempBody); + $body = $tempBody; + } + + // Cast it to string + $body = (string) $body; + + // Parse just if is not empty + if (!empty($body)) { $this->parseBody($this->parseXml($body, $response), $data); } else { $this->parseHeaders($response, $data); @@ -97,15 +109,27 @@ protected function registerNamespacePrefix(\SimpleXMLElement $element) } protected function payload( - ResponseInterface $response, + ResponseInterface|\SimpleXMLElement|array $responseOrParsedBody, StructureShape $member ) { - $xmlBody = $this->parseXml($response->getBody(), $response); + $xmlBody = $responseOrParsedBody; + if ($responseOrParsedBody instanceof ResponseInterface) { + $xmlBody = $this->parseXml( + $responseOrParsedBody->getBody(), + $responseOrParsedBody + ); + } + + $prefix = $this->registerNamespacePrefix($xmlBody); $errorBody = $xmlBody->xpath("//{$prefix}Error"); if (is_array($errorBody) && !empty($errorBody[0])) { return $this->parser->parse($member, $errorBody[0]); } + + throw new ParserException( + "Error element not found in parsed body" + ); } } diff --git a/src/Api/Operation.php b/src/Api/Operation.php index 36ba3950cb..43c695e72b 100644 --- a/src/Api/Operation.php +++ b/src/Api/Operation.php @@ -89,7 +89,7 @@ public function getOutput() /** * Get an array of operation error shapes. * - * @return Shape[] + * @return StructureShape[] */ public function getErrors() { diff --git a/src/Api/Parser/QueryParser.php b/src/Api/Parser/QueryParser.php index 2ea0676675..5b94ce6907 100644 --- a/src/Api/Parser/QueryParser.php +++ b/src/Api/Parser/QueryParser.php @@ -41,7 +41,7 @@ public function __invoke( ) { $output = $this->api->getOperation($command->getName())->getOutput(); $body = $response->getBody(); - $xml = !$body->isSeekable() || $body->getSize() + $xml = (!$body->isSeekable() || $body->getSize()) ? $this->parseXml($body, $response) : null; diff --git a/src/AwsClient.php b/src/AwsClient.php index 2b860b2211..b29c1ac061 100644 --- a/src/AwsClient.php +++ b/src/AwsClient.php @@ -283,6 +283,7 @@ public function __construct(array $args) $args['with_resolved']($config); } $this->addUserAgentMiddleware($config); + $this->addEventStreamHttpFlagMiddleware(); } public function getHandlerList() @@ -643,6 +644,33 @@ private function addUserAgentMiddleware($args) ); } + /** + * Enables streaming the response by using the stream flag. + * + * @return void + */ + private function addEventStreamHttpFlagMiddleware(): void + { + $this->getHandlerList() + -> appendInit( + function (callable $handler) { + return function (CommandInterface $command, $request = null) use ($handler) { + $operation = $this->getApi()->getOperation($command->getName()); + $output = $operation->getOutput(); + foreach ($output->getMembers() as $memberProps) { + if (!empty($memberProps['eventstream'])) { + $command['@http']['stream'] = true; + break; + } + } + + return $handler($command, $request); + }; + }, + 'event-streaming-flag-middleware' + ); + } + /** * Retrieves client context param definition from service model, * creates mapping of client context param names with client-provided diff --git a/src/CloudWatchLogs/CloudWatchLogsClient.php b/src/CloudWatchLogs/CloudWatchLogsClient.php index 8023c0029a..35034e4d71 100644 --- a/src/CloudWatchLogs/CloudWatchLogsClient.php +++ b/src/CloudWatchLogs/CloudWatchLogsClient.php @@ -199,29 +199,6 @@ class CloudWatchLogsClient extends AwsClient { public function __construct(array $args) { parent::__construct($args); - $this->addStreamingFlagMiddleware(); - } - - private function addStreamingFlagMiddleware() - { - $this->getHandlerList() - -> appendInit( - $this->getStreamingFlagMiddleware(), - 'streaming-flag-middleware' - ); - } - - private function getStreamingFlagMiddleware(): callable - { - return function (callable $handler) { - return function (CommandInterface $command, $request = null) use ($handler) { - if (!empty(self::$streamingCommands[$command->getName()])) { - $command['@http']['stream'] = true; - } - - return $handler($command, $request); - }; - }; } /** diff --git a/src/WrappedHttpHandler.php b/src/WrappedHttpHandler.php index 1c602de494..c6cfa3260c 100644 --- a/src/WrappedHttpHandler.php +++ b/src/WrappedHttpHandler.php @@ -166,10 +166,11 @@ private function parseError( throw new \RuntimeException('The HTTP handler was rejected without an "exception" key value pair.'); } - $serviceError = "AWS HTTP error: " . $err['exception']->getMessage(); + $serviceError = "AWS HTTP error:\n"; if (!isset($err['response'])) { $parts = ['response' => null]; + $serviceError .= $err['exception']->getMessage(); } else { try { $parts = call_user_func( @@ -177,8 +178,9 @@ private function parseError( $err['response'], $command ); - $serviceError .= " {$parts['code']} ({$parts['type']}): " - . "{$parts['message']} - " . $err['response']->getBody(); + + $serviceError .= "{$parts['code']} ({$parts['type']}): " + . "{$parts['message']}"; } catch (ParserException $e) { $parts = []; $serviceError .= ' Unable to parse error information from ' diff --git a/tests/AwsClientTest.php b/tests/AwsClientTest.php index 2f612faaf5..96134647d5 100644 --- a/tests/AwsClientTest.php +++ b/tests/AwsClientTest.php @@ -3,6 +3,7 @@ use Aws\Api\ApiProvider; use Aws\Api\ErrorParser\JsonRpcErrorParser; +use Aws\Api\Service; use Aws\AwsClient; use Aws\CommandInterface; use Aws\Credentials\Credentials; @@ -22,6 +23,7 @@ use Aws\Token\Token; use Aws\Waiter; use Aws\WrappedHttpHandler; +use Exception; use GuzzleHttp\Promise\PromiseInterface; use GuzzleHttp\Promise\RejectedPromise; use GuzzleHttp\Psr7\Response; @@ -99,14 +101,14 @@ public function testReturnsCommandForOperation() public function testWrapsExceptions() { - $this->expectExceptionMessage("Error executing \"foo\" on \"http://us-east-1.foo.amazonaws.com/\"; AWS HTTP error: Baz Bar!"); - $this->expectException(\Aws\S3\Exception\S3Exception::class); + $this->expectExceptionMessage("Error executing \"foo\" on \"http://us-east-1.foo.amazonaws.com/\"; AWS HTTP error:\nBaz Bar!"); + $this->expectException(S3Exception::class); $parser = function () {}; $errorParser = new JsonRpcErrorParser(); $h = new WrappedHttpHandler( function () { return new RejectedPromise([ - 'exception' => new \Exception('Baz Bar!'), + 'exception' => new Exception('Baz Bar!'), 'connection_error' => true, 'response' => null ]); @@ -1021,4 +1023,137 @@ public function testAppendsUserAgentMiddleware() ]); $client->listBuckets(); } + + /** + * @dataProvider appendEventStreamFlagMiddlewareProvider + * + * @param array $definition + * @param bool $isFlagPresent + * + * @return void + * + * @throws Exception + */ + public function testAppendEventStreamHttpFlagMiddleware( + array $definition, + bool $isFlagPresent + ): void + { + + $service = new Service($definition, function () {}); + $client = new AwsClient([ + 'service' => 'TestService', + 'api_provider' => function () use ($service) { + return $service->toArray(); + }, + 'region' => 'us-east-1', + 'version' => 'latest', + ]); + $called = false; + $client->getHandlerList()->setHandler(new MockHandler([new Result()])); + $client->getHandlerList()->appendInit(function(callable $handler) + use ($isFlagPresent, &$called) { + return function (CommandInterface $command, RequestInterface $request = null) + use ($handler, $isFlagPresent, &$called) { + $called = true; + $this->assertTrue( + ($command['@http']['stream'] ?? false) === $isFlagPresent, + ); + + return $handler($command, $request); + }; + }); + + $command = $client->getCommand('OperationTest'); + $client->execute($command); + $this->assertTrue($called); + } + + /** + * @return array[] + */ + public function appendEventStreamFlagMiddlewareProvider(): array + { + return [ + 'service_with_flag_present' => [ + 'definition' => [ + 'metadata' => [ + 'protocol' => 'rest-json', + 'protocols' => [ + 'rest-json' + ] + ], + 'operations' => [ + 'OperationTest' => [ + 'name' => 'OperationTest', + 'http' => [ + 'method' => 'POST', + 'requestUri' => '/operationTest', + 'responseCode' => 200, + ], + 'input' => ['shape' => 'OperationTestInput'], + 'output' => ['shape' => 'OperationTestOutput'], + ] + ], + 'shapes' => [ + 'OperationTestInput' => [ + 'type' => 'structure', + 'members' => [ + ] + ], + 'OperationTestOutput' => [ + 'type' => 'structure', + 'members' => [ + 'Stream' => [ + 'shape' => 'StreamShape', + 'eventstream' => true + ] + ] + ], + 'StreamShape' => ['type' => 'structure'] + ] + ], + 'present' => true + ], + 'service_with_flag_no_present' => [ + 'definition' => [ + 'metadata' => [ + 'protocol' => 'rest-json', + 'protocols' => [ + 'rest-json' + ] + ], + 'operations' => [ + 'OperationTest' => [ + 'name' => 'OperationTest', + 'http' => [ + 'method' => 'POST', + 'requestUri' => '/operationTest', + 'responseCode' => 200, + ], + 'input' => ['shape' => 'OperationTestInput'], + 'output' => ['shape' => 'OperationTestOutput'], + ] + ], + 'shapes' => [ + 'OperationTestInput' => [ + 'type' => 'structure', + 'members' => [ + ] + ], + 'OperationTestOutput' => [ + 'type' => 'structure', + 'members' => [ + 'NoStream' => [ + 'shape' => 'NoStreamShape', + ] + ] + ], + 'NoStreamShape' => ['type' => 'structure'] + ] + ], + 'present' => false + ] + ]; + } } diff --git a/tests/WrappedHttpHandlerTest.php b/tests/WrappedHttpHandlerTest.php index 9311b47ae6..f813f4089b 100644 --- a/tests/WrappedHttpHandlerTest.php +++ b/tests/WrappedHttpHandlerTest.php @@ -11,6 +11,8 @@ use Aws\Result; use Aws\WrappedHttpHandler; use GuzzleHttp\Promise\RejectedPromise; +use GuzzleHttp\Psr7\NoSeekStream; +use GuzzleHttp\Psr7\Utils; use Psr\Http\Message\RequestInterface; use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; @@ -375,4 +377,84 @@ public function testPassesOnTransferStatsCallbackToHandlerWhenRequested() $wrapped(new Command('a'), new Request('GET', 'http://foo.com')) ->wait(); } + + /** + * @dataProvider errorIsParsedOnNonSeekableResponseBodyProvider + * + * @return void + */ + public function testErrorIsParsedOnNonSeekableResponseBody( + string $protocol, + string $body, + string $expected + ) + { + $service = $this->generateTestService($protocol); + $parser = Service::createParser($service); + $errorParser = Service::createErrorParser($service->getProtocol(), $service); + $client = $this->generateTestClient( + $service + ); + $command = $client->getCommand('TestOperation'); + $exception = new AwsException( + 'Failed performing test operation', + $command, + ); + $uri = 'http://myservice.myregion.foo.com'; + $request = new Request('GET', $uri); + $response = new Response( + 403, + [], + new NoSeekStream( + Utils::streamFor($body) + ) + ); + $handler = function () use ($exception, $response) { + return new RejectedPromise([ + 'exception' => $exception, + 'response' => $response, + ]); + }; + $wrapped = new WrappedHttpHandler($handler, $parser, $errorParser); + try { + $wrapped($command, $request)->wait(); + $this->fail( + "Operation should have failed!" + ); + } catch (\Exception $exception) { + $this->assertStringContainsString( + $expected, + $exception->getMessage() + ); + } + } + + /** + * @return array[] + */ + public function errorIsParsedOnNonSeekableResponseBodyProvider(): array + { + return [ + 'json' => [ + 'protocol' => 'json', + 'body' => '{"Message": "Action not allowed!", "__Type": "ListObjects"}', + 'expected' => 'ListObjects (client): Action not allowed!', + ], + 'query' => [ + 'protocol' => 'query', + 'body' => 'ListObjectsAction not allowed!', + 'expected' => 'ListObjects (client): Action not allowed!', + ], + 'rest-xml' => [ + 'protocol' => 'rest-xml', + 'body' => 'ListObjectsAction not allowed!', + 'expected' => 'ListObjects (client): Action not allowed!', + ], + 'rest-json' => [ + 'protocol' => 'rest-json', + 'body' => '{"message": "Action not allowed!", "code": "ListObjects"}', + 'expected' => 'ListObjects (client): Action not allowed!', + ] + ]; + } }