From 5dde428fb6ca1b83dc264fe16ba44f5dac19aed4 Mon Sep 17 00:00:00 2001 From: Paul Klimov Date: Mon, 20 Mar 2023 15:52:03 +0200 Subject: [PATCH 01/13] #81: add `JsonRpcResponseNormalizer::$debug` allowing rendering error trace if enabled --- composer.json | 1 + .../JsonRpcResponseNormalizer.php | 30 ++++++++++- src/Domain/Exception/JsonRpcException.php | 19 +++++-- .../Exception/JsonRpcExceptionInterface.php | 2 +- .../JsonRpcInternalErrorException.php | 12 +++-- .../JsonRpcResponseNormalizerTest.php | 52 +++++++++++++++++++ .../Domain/Exception/JsonRpcExceptionTest.php | 2 +- .../JsonRpcInternalErrorExceptionTest.php | 12 ++--- .../JsonRpcInvalidParamsExceptionTest.php | 2 +- .../JsonRpcInvalidRequestExceptionTest.php | 2 +- .../JsonRpcMethodNotFoundExceptionTest.php | 2 +- .../JsonRpcParseErrorExceptionTest.php | 2 +- .../App/Creator/ResponseCreatorTest.php | 6 +-- 13 files changed, 115 insertions(+), 29 deletions(-) diff --git a/composer.json b/composer.json index 7a7a438c..3525f066 100644 --- a/composer.json +++ b/composer.json @@ -39,6 +39,7 @@ "behat/behat": "~3.0", "squizlabs/php_codesniffer": "3.*", "phpunit/phpunit": "^7.0 || ^8.0", + "phpspec/prophecy": "^1.0", "yoanm/php-unit-extended": "^1.0", "yoanm/jsonrpc-server-doc-sdk": "^0.2" } diff --git a/src/App/Serialization/JsonRpcResponseNormalizer.php b/src/App/Serialization/JsonRpcResponseNormalizer.php index b73070e5..ed9dd3aa 100644 --- a/src/App/Serialization/JsonRpcResponseNormalizer.php +++ b/src/App/Serialization/JsonRpcResponseNormalizer.php @@ -18,6 +18,16 @@ class JsonRpcResponseNormalizer const SUB_KEY_ERROR_MESSAGE = 'message'; const SUB_KEY_ERROR_DATA = 'data'; + /** + * @var bool whether to display debug data for the errors. + */ + protected $debug = false; + + public function __construct(bool $debug = false) + { + $this->debug = $debug; + } + /** * @param JsonRpcResponse $response * @@ -58,10 +68,26 @@ private function normalizeError(JsonRpcExceptionInterface $error) : array self::SUB_KEY_ERROR_MESSAGE => $error->getErrorMessage() ]; - if ($error->getErrorData()) { - $normalizedError[self::SUB_KEY_ERROR_DATA] = $error->getErrorData(); + $errorData = $error->getErrorData(); + + if ($this->debug) { + $errorData += $this->composeDebugErrorData($error->getPrevious() ?? $error); + } + + + if (!empty($errorData)) { + $normalizedError[self::SUB_KEY_ERROR_DATA] = $errorData; } return $normalizedError; } + + private function composeDebugErrorData(\Throwable $error) : array + { + return [ + '_code' => $error->getCode(), + '_message' => $error->getMessage(), + '_trace' => $error->getTrace(), + ]; + } } diff --git a/src/Domain/Exception/JsonRpcException.php b/src/Domain/Exception/JsonRpcException.php index a408208a..d4dcfd1a 100644 --- a/src/Domain/Exception/JsonRpcException.php +++ b/src/Domain/Exception/JsonRpcException.php @@ -2,7 +2,12 @@ namespace Yoanm\JsonRpcServer\Domain\Exception; /** - * Class JsonRpcException + * JsonRpcException represents an error which can be rendered as a JsonRpc response. + * It allows the specification of the particular data to be displayed making error more verbose. + * + * > Note: be careful about the data you expose via this mechanism, make sure you do not expose any vital information through it. + * + * @see \Yoanm\JsonRpcServer\App\Serialization\JsonRpcResponseNormalizer */ class JsonRpcException extends \Exception implements JsonRpcExceptionInterface { @@ -14,25 +19,31 @@ class JsonRpcException extends \Exception implements JsonRpcExceptionInterface * @param string $message * @param array $data */ - public function __construct(int $code, string $message = '', array $data = []) + public function __construct(int $code, string $message = '', array $data = [], ?\Throwable $previous = null) { $this->data = $data; - parent::__construct($message, $code); + parent::__construct($message, $code, $previous); } + /** + * {@inheritdoc} + */ public function getErrorCode() : int { return parent::getCode(); } + /** + * {@inheritdoc} + */ public function getErrorMessage() : string { return parent::getMessage(); } /** - * @return array + * {@inheritdoc} */ public function getErrorData() : array { diff --git a/src/Domain/Exception/JsonRpcExceptionInterface.php b/src/Domain/Exception/JsonRpcExceptionInterface.php index 1a2cb41a..2716bd1e 100644 --- a/src/Domain/Exception/JsonRpcExceptionInterface.php +++ b/src/Domain/Exception/JsonRpcExceptionInterface.php @@ -4,7 +4,7 @@ /** * Interface JsonRpcExceptionInterface */ -interface JsonRpcExceptionInterface +interface JsonRpcExceptionInterface extends \Throwable { /** * @return int JsonRpc error code diff --git a/src/Domain/Exception/JsonRpcInternalErrorException.php b/src/Domain/Exception/JsonRpcInternalErrorException.php index 3dad48ef..5c0b5f7a 100644 --- a/src/Domain/Exception/JsonRpcInternalErrorException.php +++ b/src/Domain/Exception/JsonRpcInternalErrorException.php @@ -2,23 +2,27 @@ namespace Yoanm\JsonRpcServer\Domain\Exception; /** - * Class JsonRpcInternalErrorException + * JsonRpcInternalErrorException represents unhandled error during JsonRpc method processing (e.g. "Internal server error"). */ class JsonRpcInternalErrorException extends JsonRpcException { const CODE = -32603; + /** + * @deprecated no longer in use, will be removed at next major release. + */ const DATA_PREVIOUS_KEY = 'previous'; /** - * @param \Exception|null $previousException + * @param \Throwable|null $previousException */ - public function __construct(\Exception $previousException = null) + public function __construct(\Throwable $previousException = null) { parent::__construct( self::CODE, 'Internal error', - $previousException ? [self::DATA_PREVIOUS_KEY => $previousException->getMessage()] : [] + [], + $previousException ); } } diff --git a/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php b/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php index 5ac33e18..0d975486 100644 --- a/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php +++ b/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php @@ -4,6 +4,7 @@ use PHPUnit\Framework\TestCase; use Yoanm\JsonRpcServer\App\Serialization\JsonRpcResponseNormalizer; use Yoanm\JsonRpcServer\Domain\Exception\JsonRpcException; +use Yoanm\JsonRpcServer\Domain\Exception\JsonRpcInternalErrorException; use Yoanm\JsonRpcServer\Domain\Model\JsonRpcResponse; /** @@ -132,4 +133,55 @@ public function testShouldNormalizeErrorWithData() $this->assertArrayHasKey(self::EXPECTED_SUB_KEY_ERROR_DATA, $errorObject, 'Error data not found'); $this->assertSame($data, $errorObject[self::EXPECTED_SUB_KEY_ERROR_DATA], 'Error data not expected'); } + + public function testShouldConcealErrorDataWithoutDebug() + { + $this->responseNormalizer = new JsonRpcResponseNormalizer(false); + + $exceptionMessage = 'Test exception'; + $exceptionCode = 12345; + + try { + throw new \RuntimeException($exceptionMessage, $exceptionCode); + } catch (\Throwable $exception) { + // shutdown test exception as prepared + } + + $response = (new JsonRpcResponse()) + ->setError(new JsonRpcInternalErrorException($exception)); + + $result = $this->responseNormalizer->normalize($response); + + $this->assertTrue(empty($result[self::EXPECTED_KEY_ERROR][self::EXPECTED_SUB_KEY_ERROR_DATA])); + } + + public function testShouldShowErrorDataWithDebug() + { + $this->responseNormalizer = new JsonRpcResponseNormalizer(true); + + $exceptionMessage = 'Test exception'; + $exceptionCode = 12345; + + try { + throw new \RuntimeException($exceptionMessage, $exceptionCode); + } catch (\Throwable $exception) { + // shutdown test exception as prepared + } + + $response = (new JsonRpcResponse()) + ->setError(new JsonRpcInternalErrorException($exception)); + + $result = $this->responseNormalizer->normalize($response); + + $this->assertFalse(empty($result[self::EXPECTED_KEY_ERROR][self::EXPECTED_SUB_KEY_ERROR_DATA])); + + $debugData = $result[self::EXPECTED_KEY_ERROR][self::EXPECTED_SUB_KEY_ERROR_DATA]; + + $this->assertFalse(empty($debugData['_code'])); + $this->assertFalse(empty($debugData['_message'])); + $this->assertFalse(empty($debugData['_trace'])); + + $this->assertSame($exceptionMessage, $debugData['_message']); + $this->assertSame($exceptionCode, $debugData['_code']); + } } diff --git a/tests/Functional/Domain/Exception/JsonRpcExceptionTest.php b/tests/Functional/Domain/Exception/JsonRpcExceptionTest.php index b01bf41d..f1167507 100644 --- a/tests/Functional/Domain/Exception/JsonRpcExceptionTest.php +++ b/tests/Functional/Domain/Exception/JsonRpcExceptionTest.php @@ -1,5 +1,5 @@ assertArrayHasKey( - JsonRpcInternalErrorException::DATA_PREVIOUS_KEY, - $exception->getErrorData() - ); - $this->assertSame( - $message, - $exception->getErrorData()[JsonRpcInternalErrorException::DATA_PREVIOUS_KEY] - ); + $this->assertEmpty($exception->getErrorData()); + $this->assertSame($previousException, $exception->getPrevious()); } } diff --git a/tests/Functional/Domain/Exception/JsonRpcInvalidParamsExceptionTest.php b/tests/Functional/Domain/Exception/JsonRpcInvalidParamsExceptionTest.php index 6fcdc708..7250d9d3 100644 --- a/tests/Functional/Domain/Exception/JsonRpcInvalidParamsExceptionTest.php +++ b/tests/Functional/Domain/Exception/JsonRpcInvalidParamsExceptionTest.php @@ -1,5 +1,5 @@ responseCreator->createErrorResponse($exception); $this->assertInstanceOf(JsonRpcInternalErrorException::class, $response->getError()); - $this->assertSame( - $message, - $response->getError()->getErrorData()[JsonRpcInternalErrorException::DATA_PREVIOUS_KEY] - ); + $this->assertEmpty($response->getError()->getErrorData()); + $this->assertSame($exception, $response->getError()->getPrevious()); } } From 44f953eea690ee3d82d8f91de66e30c36284b966 Mon Sep 17 00:00:00 2001 From: Paul Klimov Date: Thu, 23 Mar 2023 17:52:08 +0200 Subject: [PATCH 02/13] #81: extract `JsonRpcResponseErrorNormalizer` for error debug rendering control --- .../JsonRpcResponseErrorNormalizer.php | 151 ++++++++++++++++++ .../JsonRpcResponseNormalizer.php | 23 +-- .../JsonRpcResponseErrorNormalizerTest.php | 121 ++++++++++++++ .../JsonRpcResponseNormalizerTest.php | 18 +-- 4 files changed, 283 insertions(+), 30 deletions(-) create mode 100644 src/App/Serialization/JsonRpcResponseErrorNormalizer.php create mode 100644 tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php diff --git a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php new file mode 100644 index 00000000..93b86307 --- /dev/null +++ b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php @@ -0,0 +1,151 @@ +maxTraceSize = $maxTraceSize; + $this->showTraceArguments = $showTraceArguments; + $this->simplifyTraceArguments = $simplifyTraceArguments; + } + + /** + * @param JsonRpcExceptionInterface $error + * @return array + */ + public function normalize(JsonRpcExceptionInterface $error) : array + { + return $this->composeDebugErrorData($error->getPrevious() ?? $error); + } + + /** + * @param \Throwable $error + * @return array + */ + private function composeDebugErrorData(\Throwable $error) : array + { + $data = [ + '_class' => get_class($error), + '_code' => $error->getCode(), + '_message' => $error->getMessage(), + ]; + + $trace = $this->filterErrorTrace($error->getTrace()); + if (!empty($trace)) { + $data['_trace'] = $trace; + } + + return $data; + } + + /** + * @param array $trace raw exception stack trace. + * @return array simplified stack trace. + */ + private function filterErrorTrace(array $trace): array + { + $trace = array_slice($trace, 0, $this->maxTraceSize); + + $result = []; + foreach ($trace as $entry) { + if (!empty($entry['args'])) { + if ($this->showTraceArguments) { + if ($this->simplifyTraceArguments) { + $entry['args'] = $this->simplifyArguments($entry['args']); + } + } else { + unset($entry['args']); + } + } + + $result[] = $entry; + } + + return $result; + } + + /** + * Converts arguments array to their simplified representation. + * + * @param array $args arguments array to be converted. + * @return string string representation of the arguments array. + */ + private function simplifyArguments(array $args) : string + { + $count = 0; + + $isAssoc = $args !== array_values($args); + + foreach ($args as $key => $value) { + $count++; + + if ($count >= 5) { + if ($count > 5) { + unset($args[$key]); + } else { + $args[$key] = '...'; + } + + continue; + } + + if (is_object($value)) { + $args[$key] = get_class($value); + } elseif (is_bool($value)) { + $args[$key] = $value ? 'true' : 'false'; + } elseif (is_string($value)) { + if (strlen($value) > 64) { + $args[$key] = "'" . substr($value, 0, 64) . "...'"; + } else { + $args[$key] = "'" . $value . "'"; + } + } elseif (is_array($value)) { + $args[$key] = '[' . $this->simplifyArguments($value) . ']'; + } elseif ($value===null) { + $args[$key] = 'null'; + } elseif (is_resource($value)) { + $args[$key] = 'resource'; + } + + if (is_string($key)) { + $args[$key] = "'" . $key . "' => " . $args[$key]; + } elseif($isAssoc) { + $args[$key] = $key.' => '.$args[$key]; + } + } + + return implode(', ', $args); + } +} \ No newline at end of file diff --git a/src/App/Serialization/JsonRpcResponseNormalizer.php b/src/App/Serialization/JsonRpcResponseNormalizer.php index ed9dd3aa..67b70aa5 100644 --- a/src/App/Serialization/JsonRpcResponseNormalizer.php +++ b/src/App/Serialization/JsonRpcResponseNormalizer.php @@ -18,14 +18,12 @@ class JsonRpcResponseNormalizer const SUB_KEY_ERROR_MESSAGE = 'message'; const SUB_KEY_ERROR_DATA = 'data'; - /** - * @var bool whether to display debug data for the errors. - */ - protected $debug = false; + /** @var JsonRpcResponseErrorNormalizer */ + private $responseErrorNormalizer; - public function __construct(bool $debug = false) + public function __construct(?JsonRpcResponseErrorNormalizer $responseErrorNormalizer = null) { - $this->debug = $debug; + $this->responseErrorNormalizer = $responseErrorNormalizer; } /** @@ -70,8 +68,8 @@ private function normalizeError(JsonRpcExceptionInterface $error) : array $errorData = $error->getErrorData(); - if ($this->debug) { - $errorData += $this->composeDebugErrorData($error->getPrevious() ?? $error); + if (null !== $this->responseErrorNormalizer) { + $errorData += $this->responseErrorNormalizer->normalize($error); } @@ -81,13 +79,4 @@ private function normalizeError(JsonRpcExceptionInterface $error) : array return $normalizedError; } - - private function composeDebugErrorData(\Throwable $error) : array - { - return [ - '_code' => $error->getCode(), - '_message' => $error->getMessage(), - '_trace' => $error->getTrace(), - ]; - } } diff --git a/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php new file mode 100644 index 00000000..9c39541e --- /dev/null +++ b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php @@ -0,0 +1,121 @@ +prepareException($exceptionMessage, $exceptionCode); + + $debugData = $normalizer->normalize(new JsonRpcInternalErrorException($exception)); + + $this->assertFalse(empty($debugData['_class'])); + $this->assertFalse(empty($debugData['_code'])); + $this->assertFalse(empty($debugData['_message'])); + $this->assertFalse(empty($debugData['_trace'])); + + $this->assertSame(get_class($exception), $debugData['_class']); + $this->assertSame($exceptionMessage, $debugData['_message']); + $this->assertSame($exceptionCode, $debugData['_code']); + } + + /** + * @depends testShouldNormalizeError + */ + public function testShouldRestrictTraceSize() + { + $exception = $this->prepareException(); + $maxTraceSize = 1; + + $normalizer = new JsonRpcResponseErrorNormalizer($maxTraceSize); + $debugData = $normalizer->normalize(new JsonRpcInternalErrorException($exception)); + + $this->assertCount($maxTraceSize, $debugData['_trace']); + } + + /** + * @depends testShouldNormalizeError + */ + public function testShouldNotDisplayTraceOnZeroSize() + { + $exception = $this->prepareException(); + + $normalizer = new JsonRpcResponseErrorNormalizer(0); + $debugData = $normalizer->normalize(new JsonRpcInternalErrorException($exception)); + + $this->assertFalse(array_key_exists('_trace', $debugData)); + } + + /** + * @depends testShouldRestrictTraceSize + */ + public function testShouldShowTraceArguments() + { + $exception = $this->prepareException(); + + $normalizer = new JsonRpcResponseErrorNormalizer(99, true); + $debugData = $normalizer->normalize(new JsonRpcInternalErrorException($exception)); + + $argsFound = false; + foreach ($debugData['_trace'] as $entry) { + if (isset($entry['args'])) { + $argsFound = true; + break; + } + } + + $this->assertTrue($argsFound); + } + + /** + * @depends testShouldRestrictTraceSize + */ + public function testShouldHideTraceArguments() + { + $exception = $this->prepareException(); + + $normalizer = new JsonRpcResponseErrorNormalizer(99, false); + $debugData = $normalizer->normalize(new JsonRpcInternalErrorException($exception)); + + $argsFound = false; + foreach ($debugData['_trace'] as $entry) { + if (isset($entry['args'])) { + $argsFound = true; + break; + } + } + + $this->assertFalse($argsFound); + } +} \ No newline at end of file diff --git a/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php b/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php index 0d975486..3b83d157 100644 --- a/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php +++ b/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php @@ -2,6 +2,7 @@ namespace Tests\Functional\App\Serialization; use PHPUnit\Framework\TestCase; +use Yoanm\JsonRpcServer\App\Serialization\JsonRpcResponseErrorNormalizer; use Yoanm\JsonRpcServer\App\Serialization\JsonRpcResponseNormalizer; use Yoanm\JsonRpcServer\Domain\Exception\JsonRpcException; use Yoanm\JsonRpcServer\Domain\Exception\JsonRpcInternalErrorException; @@ -134,9 +135,9 @@ public function testShouldNormalizeErrorWithData() $this->assertSame($data, $errorObject[self::EXPECTED_SUB_KEY_ERROR_DATA], 'Error data not expected'); } - public function testShouldConcealErrorDataWithoutDebug() + public function testShouldConcealErrorDataWithoutErrorNormalizer() { - $this->responseNormalizer = new JsonRpcResponseNormalizer(false); + $this->responseNormalizer = new JsonRpcResponseNormalizer(); $exceptionMessage = 'Test exception'; $exceptionCode = 12345; @@ -155,9 +156,9 @@ public function testShouldConcealErrorDataWithoutDebug() $this->assertTrue(empty($result[self::EXPECTED_KEY_ERROR][self::EXPECTED_SUB_KEY_ERROR_DATA])); } - public function testShouldShowErrorDataWithDebug() + public function testShouldShowErrorDataWithErrorNormalizer() { - $this->responseNormalizer = new JsonRpcResponseNormalizer(true); + $this->responseNormalizer = new JsonRpcResponseNormalizer(new JsonRpcResponseErrorNormalizer()); $exceptionMessage = 'Test exception'; $exceptionCode = 12345; @@ -174,14 +175,5 @@ public function testShouldShowErrorDataWithDebug() $result = $this->responseNormalizer->normalize($response); $this->assertFalse(empty($result[self::EXPECTED_KEY_ERROR][self::EXPECTED_SUB_KEY_ERROR_DATA])); - - $debugData = $result[self::EXPECTED_KEY_ERROR][self::EXPECTED_SUB_KEY_ERROR_DATA]; - - $this->assertFalse(empty($debugData['_code'])); - $this->assertFalse(empty($debugData['_message'])); - $this->assertFalse(empty($debugData['_trace'])); - - $this->assertSame($exceptionMessage, $debugData['_message']); - $this->assertSame($exceptionCode, $debugData['_code']); } } From 2bfe8aadd96614fbcdcec105af74881e778dce9c Mon Sep 17 00:00:00 2001 From: Paul Klimov Date: Mon, 27 Mar 2023 10:51:14 +0300 Subject: [PATCH 03/13] #81: improve condition for trace arguments presence --- src/App/Serialization/JsonRpcResponseErrorNormalizer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php index 93b86307..63802245 100644 --- a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php +++ b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php @@ -80,7 +80,7 @@ private function filterErrorTrace(array $trace): array $result = []; foreach ($trace as $entry) { - if (!empty($entry['args'])) { + if (array_key_exists('args', $entry)) { if ($this->showTraceArguments) { if ($this->simplifyTraceArguments) { $entry['args'] = $this->simplifyArguments($entry['args']); From bd3a7096d67ac416ab42f915c0234173d3f6e738 Mon Sep 17 00:00:00 2001 From: Yoanm Date: Sat, 1 Apr 2023 15:10:48 +0200 Subject: [PATCH 04/13] Fix FTs --- features/event_actions.feature | 10 ++-------- features/json-rpc-specs/built-in-errors.feature | 5 +---- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/features/event_actions.feature b/features/event_actions.feature index 4c7e3e39..92d4e11c 100644 --- a/features/event_actions.feature +++ b/features/event_actions.feature @@ -43,10 +43,7 @@ Feature: Actions through events "id": null, "error": { "code": -32603, - "message": "Internal error", - "data": { - "previous": "my custom exception message" - } + "message": "Internal error" } } """ @@ -94,10 +91,7 @@ Feature: Actions through events "id": 1, "error": { "code": -32603, - "message": "Internal error", - "data": { - "previous": "my custom exception message" - } + "message": "Internal error" } } """ diff --git a/features/json-rpc-specs/built-in-errors.feature b/features/json-rpc-specs/built-in-errors.feature index e8b187b3..d7dbf4c8 100644 --- a/features/json-rpc-specs/built-in-errors.feature +++ b/features/json-rpc-specs/built-in-errors.feature @@ -124,10 +124,7 @@ Feature: Ensure JSON-RPC errors specifications "id": "297c8498-5a54-471c-ac75-917be6435607", "error": { "code": -32603, - "message": "Internal error", - "data": { - "previous": "method-that-throw-an-exception-during-execution execution exception" - } + "message": "Internal error" } } """ From 118d0ab4f04258c2abf50a1663d1196a036ed168 Mon Sep 17 00:00:00 2001 From: Yoanm Date: Sat, 1 Apr 2023 15:11:09 +0200 Subject: [PATCH 05/13] Add FT with debug mode enabled --- features/bootstrap/FeatureContext.php | 14 +++++++++- .../bootstrap/Helper/FakeEndpointCreator.php | 13 +++++++-- .../json-rpc-specs/built-in-errors.feature | 28 +++++++++++++++++++ 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/features/bootstrap/FeatureContext.php b/features/bootstrap/FeatureContext.php index 3dbfe0e6..a92e8990 100644 --- a/features/bootstrap/FeatureContext.php +++ b/features/bootstrap/FeatureContext.php @@ -4,6 +4,7 @@ use Behat\Behat\Context\Environment\InitializedContextEnvironment; use Behat\Behat\Hook\Scope\BeforeScenarioScope; use Behat\Gherkin\Node\PyStringNode; +use DemoApp\Dispatcher\BehatRequestLifecycleDispatcher; use PHPUnit\Framework\Assert; use Tests\Functional\BehatContext\Helper\FakeEndpointCreator; @@ -26,6 +27,8 @@ class FeatureContext extends AbstractContext /** @var EventsContext */ private $eventsContext; + private bool $enableJsonRpcResponseErrorNormalizer = false; + /** * @BeforeScenario * @@ -40,12 +43,21 @@ public function beforeScenario(BeforeScenarioScope $scope) } } + /** + * @Given JsonRpcResponseErrorNormalizer is enabled + */ + public function givenJsonRpcResponseErrorNormalizerIsEnabled() + { + $this->enableJsonRpcResponseErrorNormalizer = true; + } + /** * @When I send following payload: */ public function whenISendTheFollowingPayload(PyStringNode $payload) { - $endpoint = (new FakeEndpointCreator())->create($this->eventsContext->getDispatcher()); + $endpoint = (new FakeEndpointCreator()) + ->create($this->eventsContext->getDispatcher(), $this->enableJsonRpcResponseErrorNormalizer); $this->lastResponse = $endpoint->index($payload->getRaw()); } diff --git a/features/bootstrap/Helper/FakeEndpointCreator.php b/features/bootstrap/Helper/FakeEndpointCreator.php index 46d5707e..a1774c4d 100644 --- a/features/bootstrap/Helper/FakeEndpointCreator.php +++ b/features/bootstrap/Helper/FakeEndpointCreator.php @@ -15,6 +15,7 @@ use Yoanm\JsonRpcServer\App\Serialization\JsonRpcCallResponseNormalizer; use Yoanm\JsonRpcServer\App\Serialization\JsonRpcCallSerializer; use Yoanm\JsonRpcServer\App\Serialization\JsonRpcRequestDenormalizer; +use Yoanm\JsonRpcServer\App\Serialization\JsonRpcResponseErrorNormalizer; use Yoanm\JsonRpcServer\App\Serialization\JsonRpcResponseNormalizer; use Yoanm\JsonRpcServer\Domain\JsonRpcMethodInterface; use Yoanm\JsonRpcServer\Domain\JsonRpcMethodParamsValidatorInterface; @@ -27,8 +28,10 @@ class FakeEndpointCreator /** * @return JsonRpcEndpoint */ - public function create(JsonRpcServerDispatcherInterface $dispatcher = null) : JsonRpcEndpoint - { + public function create( + JsonRpcServerDispatcherInterface $dispatcher = null, + bool $enableJsonRpcResponseErrorNormalizer = false + ) : JsonRpcEndpoint { /** @var AbstractMethod[] $methodList */ $methodList = [ 'basic-method' => new BasicMethod(), @@ -37,6 +40,10 @@ public function create(JsonRpcServerDispatcherInterface $dispatcher = null) : Js 'method-that-throw-an-exception-during-execution' => new MethodThatThrowExceptionDuringExecution(), 'method-that-throw-a-custom-jsonrpc-exception-during-execution' => new MethodThatThrowJsonRpcExceptionDuringExecution(), ]; + $jsonRpcResponseErrorNormalizer = $enableJsonRpcResponseErrorNormalizer + ? new JsonRpcResponseErrorNormalizer(0) + : null + ; $methodResolver = new BehatMethodResolver(); @@ -49,7 +56,7 @@ public function create(JsonRpcServerDispatcherInterface $dispatcher = null) : Js new JsonRpcRequestDenormalizer() ), new JsonRpcCallResponseNormalizer( - new JsonRpcResponseNormalizer() + new JsonRpcResponseNormalizer($jsonRpcResponseErrorNormalizer) ) ); $responseCreator = new ResponseCreator(); diff --git a/features/json-rpc-specs/built-in-errors.feature b/features/json-rpc-specs/built-in-errors.feature index d7dbf4c8..a0a38771 100644 --- a/features/json-rpc-specs/built-in-errors.feature +++ b/features/json-rpc-specs/built-in-errors.feature @@ -129,6 +129,34 @@ Feature: Ensure JSON-RPC errors specifications } """ + Scenario: Internal error (-32603) with JsonRpcResponseErrorNormalizer + Given JsonRpcResponseErrorNormalizer is enabled + When I send following payload: + """ + { + "jsonrpc": "2.0", + "id": "297c8498-5a54-471c-ac75-917be6435607", + "method": "method-that-throw-an-exception-during-execution" + } + """ + Then last response should be a valid json-rpc error + And I should have the following response: + """ + { + "jsonrpc": "2.0", + "id": "297c8498-5a54-471c-ac75-917be6435607", + "error": { + "code": -32603, + "message": "Internal error", + "data": { + "_class": "Exception", + "_code": 0, + "_message": "method-that-throw-an-exception-during-execution execution exception" + } + } + } + """ + Scenario: Implementation-defined server-errors (-32099 to -32000) When I send following payload: """ From 5ad298060059c8f6573dbdb90d99d1ec7929587e Mon Sep 17 00:00:00 2001 From: Yoanm Date: Sat, 1 Apr 2023 15:14:50 +0200 Subject: [PATCH 06/13] Lint --- src/App/Serialization/JsonRpcResponseErrorNormalizer.php | 4 ++-- .../App/Serialization/JsonRpcResponseErrorNormalizerTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php index 63802245..9be88049 100644 --- a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php +++ b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php @@ -141,11 +141,11 @@ private function simplifyArguments(array $args) : string if (is_string($key)) { $args[$key] = "'" . $key . "' => " . $args[$key]; - } elseif($isAssoc) { + } elseif ($isAssoc) { $args[$key] = $key.' => '.$args[$key]; } } return implode(', ', $args); } -} \ No newline at end of file +} diff --git a/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php index 9c39541e..f11ef122 100644 --- a/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php +++ b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php @@ -118,4 +118,4 @@ public function testShouldHideTraceArguments() $this->assertFalse($argsFound); } -} \ No newline at end of file +} From 67a42b33414e07592079ab2138fe6c2b9989da0c Mon Sep 17 00:00:00 2001 From: Yoanm Date: Sat, 1 Apr 2023 15:15:40 +0200 Subject: [PATCH 07/13] Cover all argument types conversion --- .../JsonRpcResponseErrorNormalizerTest.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php index f11ef122..c6b1ed7a 100644 --- a/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php +++ b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php @@ -15,13 +15,25 @@ class JsonRpcResponseErrorNormalizerTest extends TestCase { protected function prepareException($exceptionMessage = 'Test exception', $exceptionCode = 12345) : \Throwable { + $args = [ + 'object' => new \stdClass(), + 'bool' => true, + 'string' => 'a string', + // Managed argument count being at 5, create a sub bucket + 'sub' => [ + 'a too long string' => str_repeat('a', 100), + 'null' => null, + 'resource' => tmpfile(), + 'list' => [0, 3, 345], + ], + ]; try { // create a long stack trace - $closure = function ($exceptionMessage, $exceptionCode) { + $closure = function ($exceptionMessage, $exceptionCode, array $allTypesOfArgs) { throw new \RuntimeException($exceptionMessage, $exceptionCode); }; - call_user_func($closure, $exceptionMessage, $exceptionCode); + call_user_func($closure, $exceptionMessage, $exceptionCode, $args); } catch (\Throwable $exception) { // shutdown test exception as prepared } From 1b01952d214daba53f8d621c69f241fa17ad9c50 Mon Sep 17 00:00:00 2001 From: Yoanm Date: Sat, 1 Apr 2023 15:27:08 +0200 Subject: [PATCH 08/13] Decrease method complexity --- .../JsonRpcResponseErrorNormalizer.php | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php index 9be88049..2d44244f 100644 --- a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php +++ b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php @@ -121,23 +121,7 @@ private function simplifyArguments(array $args) : string continue; } - if (is_object($value)) { - $args[$key] = get_class($value); - } elseif (is_bool($value)) { - $args[$key] = $value ? 'true' : 'false'; - } elseif (is_string($value)) { - if (strlen($value) > 64) { - $args[$key] = "'" . substr($value, 0, 64) . "...'"; - } else { - $args[$key] = "'" . $value . "'"; - } - } elseif (is_array($value)) { - $args[$key] = '[' . $this->simplifyArguments($value) . ']'; - } elseif ($value===null) { - $args[$key] = 'null'; - } elseif (is_resource($value)) { - $args[$key] = 'resource'; - } + $args[$key] = $this->simplifyArgument($value); if (is_string($key)) { $args[$key] = "'" . $key . "' => " . $args[$key]; @@ -148,4 +132,27 @@ private function simplifyArguments(array $args) : string return implode(', ', $args); } + + private function simplifyArgument(mixed $value): mixed + { + if (is_object($value)) { + return get_class($value); + } elseif (is_bool($value)) { + return $value ? 'true' : 'false'; + } elseif (is_string($value)) { + if (strlen($value) > 64) { + return "'" . substr($value, 0, 64) . "...'"; + } else { + return "'" . $value . "'"; + } + } elseif (is_array($value)) { + return '[' . $this->simplifyArguments($value) . ']'; + } elseif ($value === null) { + return 'null'; + } elseif (is_resource($value)) { + return 'resource'; + } + + return $value; + } } From 728ed2a5cd4184163a40e6d2757edc8cb23ca819 Mon Sep 17 00:00:00 2001 From: Paul Klimov Date: Mon, 3 Apr 2023 11:31:29 +0300 Subject: [PATCH 09/13] #81: remove redundant condition --- src/App/Serialization/JsonRpcResponseErrorNormalizer.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php index 2d44244f..ccbc120c 100644 --- a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php +++ b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php @@ -47,6 +47,8 @@ public function __construct(int $maxTraceSize = 10, bool $showTraceArguments = t */ public function normalize(JsonRpcExceptionInterface $error) : array { + // `JsonRpcExceptionInterface` has a little value regarding debug error data composition on its own, + // thus use previous exception, if available: return $this->composeDebugErrorData($error->getPrevious() ?? $error); } @@ -106,8 +108,6 @@ private function simplifyArguments(array $args) : string { $count = 0; - $isAssoc = $args !== array_values($args); - foreach ($args as $key => $value) { $count++; @@ -125,7 +125,7 @@ private function simplifyArguments(array $args) : string if (is_string($key)) { $args[$key] = "'" . $key . "' => " . $args[$key]; - } elseif ($isAssoc) { + } else { $args[$key] = $key.' => '.$args[$key]; } } From c775a5e5b525089d77634f69ef20e8616b725264 Mon Sep 17 00:00:00 2001 From: Paul Klimov Date: Mon, 3 Apr 2023 15:34:36 +0300 Subject: [PATCH 10/13] #81: fix debug assoc array simple rendering --- src/App/Serialization/JsonRpcResponseErrorNormalizer.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php index ccbc120c..da2ed4c7 100644 --- a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php +++ b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php @@ -108,6 +108,8 @@ private function simplifyArguments(array $args) : string { $count = 0; + $isAssoc = $args !== array_values($args); + foreach ($args as $key => $value) { $count++; @@ -125,7 +127,8 @@ private function simplifyArguments(array $args) : string if (is_string($key)) { $args[$key] = "'" . $key . "' => " . $args[$key]; - } else { + } elseif ($isAssoc) { + // contains both numeric and string keys: $args[$key] = $key.' => '.$args[$key]; } } From f5cc760ce1eddc96ab74ae2854d0a1b386d802ef Mon Sep 17 00:00:00 2001 From: Yoanm Date: Sat, 8 Apr 2023 11:00:18 +0200 Subject: [PATCH 11/13] Improve coverage --- .../Serialization/JsonRpcResponseErrorNormalizerTest.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php index c6b1ed7a..56a17c76 100644 --- a/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php +++ b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php @@ -21,11 +21,16 @@ protected function prepareException($exceptionMessage = 'Test exception', $excep 'string' => 'a string', // Managed argument count being at 5, create a sub bucket 'sub' => [ - 'a too long string' => str_repeat('a', 100), + 'a_too_long_string' => str_repeat('a', 100), 'null' => null, 'resource' => tmpfile(), - 'list' => [0, 3, 345], + 'sub' => [ + 'list' => ['foo', 'bar'], + 'list_with_holes' => [9 => 'nine', 5 => 'five'], + 'mixed_array' => ['foo', 'name' => 'bar'] + ] ], + 'extra_param' => 'will not be normalized!' ]; try { // create a long stack trace From c1986e45f436edf5c3f7eecc1fe8951a4b4e9348 Mon Sep 17 00:00:00 2001 From: Yoanm Date: Sat, 8 Apr 2023 11:06:00 +0200 Subject: [PATCH 12/13] Update README --- README.md | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index ff48a560..fdd29fcc 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,7 @@ use Yoanm\JsonRpcServer\App\Serialization\JsonRpcCallDenormalizer; use Yoanm\JsonRpcServer\App\Serialization\JsonRpcCallResponseNormalizer; use Yoanm\JsonRpcServer\App\Serialization\JsonRpcCallSerializer; use Yoanm\JsonRpcServer\App\Serialization\JsonRpcRequestDenormalizer; +use Yoanm\JsonRpcServer\App\Serialization\JsonRpcResponseErrorNormalizer; use Yoanm\JsonRpcServer\App\Serialization\JsonRpcResponseNormalizer; use Yoanm\JsonRpcServer\Infra\Endpoint\JsonRpcEndpoint; @@ -118,7 +119,8 @@ $jsonRpcSerializer = new JsonRpcCallSerializer( new JsonRpcRequestDenormalizer() ), new JsonRpcCallResponseNormalizer( - new JsonRpcResponseNormalizer() + new JsonRpcResponseNormalizer() + // or `new JsonRpcResponseNormalizer(new JsonRpcResponseErrorNormalizer())` for debug purpose ) ); $responseCreator = new ResponseCreator(); @@ -327,21 +329,6 @@ $validator = new class implements JsonRpcMethodParamsValidatorInterface $requestHandler->setMethodParamsValidator($validator); ``` -## Makefile - -```bash -# Install and configure project -make build -# Launch tests (PHPUnit & behat) -make test -# Check project code style -make codestyle -# Generate PHPUnit coverage -make coverage -# Generate Behat coverage -make behat-coverage -``` - ## Contributing See [contributing note](./CONTRIBUTING.md) From 20b4111009c6efa02af341b6d2060553efe6f28a Mon Sep 17 00:00:00 2001 From: Yoanm Date: Sat, 8 Apr 2023 11:18:45 +0200 Subject: [PATCH 13/13] Force arguments availability on stack trace --- README.md | 3 ++- .../App/Serialization/JsonRpcResponseErrorNormalizerTest.php | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index fdd29fcc..d13ddd82 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,8 @@ $jsonRpcSerializer = new JsonRpcCallSerializer( ), new JsonRpcCallResponseNormalizer( new JsonRpcResponseNormalizer() - // or `new JsonRpcResponseNormalizer(new JsonRpcResponseErrorNormalizer())` for debug purpose + // Or `new JsonRpcResponseNormalizer(new JsonRpcResponseErrorNormalizer())` for debug purpose + // To also dump arguments, be sure 'zend.exception_ignore_args' ini option is not at true/1 ) ); $responseCreator = new ResponseCreator(); diff --git a/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php index 56a17c76..c93a4d2d 100644 --- a/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php +++ b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php @@ -99,6 +99,7 @@ public function testShouldNotDisplayTraceOnZeroSize() */ public function testShouldShowTraceArguments() { + ini_set('zend.exception_ignore_args', 0); // Be sure arguments will be available on the stack trace $exception = $this->prepareException(); $normalizer = new JsonRpcResponseErrorNormalizer(99, true); @@ -120,6 +121,7 @@ public function testShouldShowTraceArguments() */ public function testShouldHideTraceArguments() { + ini_set('zend.exception_ignore_args', 0); // Be sure arguments will be available on the stack trace $exception = $this->prepareException(); $normalizer = new JsonRpcResponseErrorNormalizer(99, false);