diff --git a/README.md b/README.md index ff48a560..d13ddd82 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,9 @@ $jsonRpcSerializer = new JsonRpcCallSerializer( new JsonRpcRequestDenormalizer() ), new JsonRpcCallResponseNormalizer( - new JsonRpcResponseNormalizer() + new JsonRpcResponseNormalizer() + // 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(); @@ -327,21 +330,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) 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/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..a0a38771 100644 --- a/features/json-rpc-specs/built-in-errors.feature +++ b/features/json-rpc-specs/built-in-errors.feature @@ -108,6 +108,29 @@ Feature: Ensure JSON-RPC errors specifications """ Scenario: Internal error (-32603) + 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" + } + } + """ + + Scenario: Internal error (-32603) with JsonRpcResponseErrorNormalizer + Given JsonRpcResponseErrorNormalizer is enabled When I send following payload: """ { @@ -126,7 +149,9 @@ Feature: Ensure JSON-RPC errors specifications "code": -32603, "message": "Internal error", "data": { - "previous": "method-that-throw-an-exception-during-execution execution exception" + "_class": "Exception", + "_code": 0, + "_message": "method-that-throw-an-exception-during-execution execution exception" } } } diff --git a/src/App/Serialization/JsonRpcResponseErrorNormalizer.php b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php new file mode 100644 index 00000000..da2ed4c7 --- /dev/null +++ b/src/App/Serialization/JsonRpcResponseErrorNormalizer.php @@ -0,0 +1,161 @@ +maxTraceSize = $maxTraceSize; + $this->showTraceArguments = $showTraceArguments; + $this->simplifyTraceArguments = $simplifyTraceArguments; + } + + /** + * @param JsonRpcExceptionInterface $error + * @return array + */ + 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); + } + + /** + * @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 (array_key_exists('args', $entry)) { + 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; + } + + $args[$key] = $this->simplifyArgument($value); + + if (is_string($key)) { + $args[$key] = "'" . $key . "' => " . $args[$key]; + } elseif ($isAssoc) { + // contains both numeric and string keys: + $args[$key] = $key.' => '.$args[$key]; + } + } + + 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; + } +} diff --git a/src/App/Serialization/JsonRpcResponseNormalizer.php b/src/App/Serialization/JsonRpcResponseNormalizer.php index b73070e5..67b70aa5 100644 --- a/src/App/Serialization/JsonRpcResponseNormalizer.php +++ b/src/App/Serialization/JsonRpcResponseNormalizer.php @@ -18,6 +18,14 @@ class JsonRpcResponseNormalizer const SUB_KEY_ERROR_MESSAGE = 'message'; const SUB_KEY_ERROR_DATA = 'data'; + /** @var JsonRpcResponseErrorNormalizer */ + private $responseErrorNormalizer; + + public function __construct(?JsonRpcResponseErrorNormalizer $responseErrorNormalizer = null) + { + $this->responseErrorNormalizer = $responseErrorNormalizer; + } + /** * @param JsonRpcResponse $response * @@ -58,8 +66,15 @@ 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 (null !== $this->responseErrorNormalizer) { + $errorData += $this->responseErrorNormalizer->normalize($error); + } + + + if (!empty($errorData)) { + $normalizedError[self::SUB_KEY_ERROR_DATA] = $errorData; } return $normalizedError; 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/JsonRpcResponseErrorNormalizerTest.php b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php new file mode 100644 index 00000000..c93a4d2d --- /dev/null +++ b/tests/Functional/App/Serialization/JsonRpcResponseErrorNormalizerTest.php @@ -0,0 +1,140 @@ + 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(), + '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 + $closure = function ($exceptionMessage, $exceptionCode, array $allTypesOfArgs) { + throw new \RuntimeException($exceptionMessage, $exceptionCode); + }; + + call_user_func($closure, $exceptionMessage, $exceptionCode, $args); + } catch (\Throwable $exception) { + // shutdown test exception as prepared + } + + return $exception; + } + + public function testShouldNormalizeError() + { + $normalizer = new JsonRpcResponseErrorNormalizer(); + + $exceptionMessage = 'Test exception'; + $exceptionCode = 12345; + + $exception = $this->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() + { + 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); + $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() + { + 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); + $debugData = $normalizer->normalize(new JsonRpcInternalErrorException($exception)); + + $argsFound = false; + foreach ($debugData['_trace'] as $entry) { + if (isset($entry['args'])) { + $argsFound = true; + break; + } + } + + $this->assertFalse($argsFound); + } +} diff --git a/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php b/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php index 767d30b7..1e937b56 100644 --- a/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php +++ b/tests/Functional/App/Serialization/JsonRpcResponseNormalizerTest.php @@ -3,8 +3,10 @@ use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; +use Yoanm\JsonRpcServer\App\Serialization\JsonRpcResponseErrorNormalizer; use Yoanm\JsonRpcServer\App\Serialization\JsonRpcResponseNormalizer; use Yoanm\JsonRpcServer\Domain\Exception\JsonRpcException; +use Yoanm\JsonRpcServer\Domain\Exception\JsonRpcInternalErrorException; use Yoanm\JsonRpcServer\Domain\Model\JsonRpcResponse; /** @@ -135,4 +137,46 @@ 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 testShouldConcealErrorDataWithoutErrorNormalizer() + { + $this->responseNormalizer = new JsonRpcResponseNormalizer(); + + $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 testShouldShowErrorDataWithErrorNormalizer() + { + $this->responseNormalizer = new JsonRpcResponseNormalizer(new JsonRpcResponseErrorNormalizer()); + + $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])); + } } diff --git a/tests/Functional/Domain/Exception/JsonRpcInternalErrorExceptionTest.php b/tests/Functional/Domain/Exception/JsonRpcInternalErrorExceptionTest.php index 50455cbb..b7bc1e79 100644 --- a/tests/Functional/Domain/Exception/JsonRpcInternalErrorExceptionTest.php +++ b/tests/Functional/Domain/Exception/JsonRpcInternalErrorExceptionTest.php @@ -28,13 +28,7 @@ public function testShouldHandleAnExceptionAnPutItInExceptionData() $exception = new JsonRpcInternalErrorException($previousException); - $this->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/Technical/App/Creator/ResponseCreatorTest.php b/tests/Technical/App/Creator/ResponseCreatorTest.php index efdb1a5e..85c6dd13 100644 --- a/tests/Technical/App/Creator/ResponseCreatorTest.php +++ b/tests/Technical/App/Creator/ResponseCreatorTest.php @@ -37,9 +37,7 @@ public function testCreateErrorResponseConvertOtherExceptions() $response = $this->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()); } }