From 21bba81635c04f85b1aba149abb6294d1bf10262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E9=93=AD=E6=98=95?= <715557344@qq.com> Date: Tue, 31 Dec 2019 12:03:50 +0800 Subject: [PATCH 1/9] Fixed bug that exception cannot be resolved successfully in TcpServer. --- src/json-rpc/src/TcpServer.php | 8 ++++ src/json-rpc/tests/TcpServerTest.php | 65 ++++++++++++++++++++++++++++ src/rpc-server/src/Server.php | 11 +++-- 3 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 src/json-rpc/tests/TcpServerTest.php diff --git a/src/json-rpc/src/TcpServer.php b/src/json-rpc/src/TcpServer.php index a5c1f259e..47e2c4fce 100644 --- a/src/json-rpc/src/TcpServer.php +++ b/src/json-rpc/src/TcpServer.php @@ -20,6 +20,7 @@ use Hyperf\HttpMessage\Server\Request as Psr7Request; use Hyperf\HttpMessage\Server\Response as Psr7Response; use Hyperf\HttpMessage\Uri\Uri; use Hyperf\HttpServer\Contract\CoreMiddlewareInterface; +use Hyperf\JsonRpc\Exception\Handler\HttpExceptionHandler; use Hyperf\Rpc\Protocol; use Hyperf\Rpc\ProtocolManager; use Hyperf\RpcServer\RequestDispatcher; @@ -144,4 +145,11 @@ class TcpServer extends Server } return $request; } + + protected function getDefaultExceptionHandler(): array + { + return [ + HttpExceptionHandler::class, + ]; + } } diff --git a/src/json-rpc/tests/TcpServerTest.php b/src/json-rpc/tests/TcpServerTest.php new file mode 100644 index 000000000..bbed3cfd9 --- /dev/null +++ b/src/json-rpc/tests/TcpServerTest.php @@ -0,0 +1,65 @@ +getContainer(); + $server = new TcpServer( + $container, + $container->get(RequestDispatcher::class), + $container->get(ExceptionHandlerDispatcher::class), + $container->get(ProtocolManager::class), + $container->get(StdoutLoggerInterface::class) + ); + + $ref = new \ReflectionClass($server); + $method = $ref->getMethod('getDefaultExceptionHandler'); + $method->setAccessible(true); + $res = $method->invoke($server); + + $this->assertSame([HttpExceptionHandler::class], $res); + } + + protected function getContainer() + { + $container = Mockery::mock(ContainerInterface::class); + ApplicationContext::setContainer($container); + + $container->shouldReceive('get')->with(RequestDispatcher::class)->andReturn(new RequestDispatcher($container)); + $container->shouldReceive('get')->with(ExceptionHandlerDispatcher::class)->andReturn(new ExceptionHandlerDispatcher($container)); + $config = new Config([]); + $container->shouldReceive('get')->with(ProtocolManager::class)->andReturn(new ProtocolManager($config)); + $container->shouldReceive('get')->with(StdoutLoggerInterface::class)->andReturn(Mockery::mock(StdoutLoggerInterface::class)); + + return $container; + } +} diff --git a/src/rpc-server/src/Server.php b/src/rpc-server/src/Server.php index c394cd701..52fed6090 100644 --- a/src/rpc-server/src/Server.php +++ b/src/rpc-server/src/Server.php @@ -96,9 +96,7 @@ abstract class Server implements OnReceiveInterface, MiddlewareInitializerInterf $config = $this->container->get(ConfigInterface::class); $this->middlewares = $config->get('middlewares.' . $serverName, []); - $this->exceptionHandlers = $config->get('exceptions.handler.' . $serverName, [ - HttpExceptionHandler::class, - ]); + $this->exceptionHandlers = $config->get('exceptions.handler.' . $serverName, $this->getDefaultExceptionHandler()); } public function onReceive(SwooleServer $server, int $fd, int $fromId, string $data): void @@ -137,6 +135,13 @@ abstract class Server implements OnReceiveInterface, MiddlewareInitializerInterf $this->logger->debug(sprintf('Connect to %s:%d', $port->host, $port->port)); } + protected function getDefaultExceptionHandler(): array + { + return [ + HttpExceptionHandler::class, + ]; + } + protected function send(SwooleServer $server, int $fd, ResponseInterface $response): void { $server->send($fd, (string) $response->getBody()); From 18bdebdefd83ef728d1cee7a308b97eb82a9ad95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E9=93=AD=E6=98=95?= <715557344@qq.com> Date: Tue, 31 Dec 2019 13:16:29 +0800 Subject: [PATCH 2/9] Fixed bug that error cannot be resolved successfully. --- src/json-rpc/src/CoreMiddleware.php | 2 +- src/json-rpc/src/DataFormatter.php | 2 +- src/json-rpc/src/NormalizeDataFormatter.php | 2 +- src/json-rpc/src/ResponseBuilder.php | 4 +- .../tests/AnyParamCoreMiddlewareTest.php | 37 +++++++++++++++++++ .../Stub/CalculatorProxyServiceClient.php | 5 +++ src/json-rpc/tests/Stub/CalculatorService.php | 11 +++--- .../tests/Stub/CalculatorServiceInterface.php | 2 + src/rpc-client/src/ServiceClient.php | 2 +- .../src/Serializer/ExceptionNormalizer.php | 10 ++--- 10 files changed, 60 insertions(+), 17 deletions(-) diff --git a/src/json-rpc/src/CoreMiddleware.php b/src/json-rpc/src/CoreMiddleware.php index 23cb57562..797334aff 100644 --- a/src/json-rpc/src/CoreMiddleware.php +++ b/src/json-rpc/src/CoreMiddleware.php @@ -46,7 +46,7 @@ class CoreMiddleware extends \Hyperf\RpcServer\CoreMiddleware $parameters = $this->parseParameters($controller, $action, $request->getParsedBody()); try { $response = $controllerInstance->{$action}(...$parameters); - } catch (\Exception $exception) { + } catch (\Throwable $exception) { $response = $this->responseBuilder->buildErrorResponse($request, ResponseBuilder::SERVER_ERROR, $exception); $this->responseBuilder->persistToContext($response); diff --git a/src/json-rpc/src/DataFormatter.php b/src/json-rpc/src/DataFormatter.php index 070dbc622..e4c96fda5 100644 --- a/src/json-rpc/src/DataFormatter.php +++ b/src/json-rpc/src/DataFormatter.php @@ -41,7 +41,7 @@ class DataFormatter implements DataFormatterInterface { [$id, $code, $message, $data] = $data; - if (isset($data) && $data instanceof \Exception) { + if (isset($data) && $data instanceof \Throwable) { $data = [ 'class' => get_class($data), 'message' => $data->getMessage(), diff --git a/src/json-rpc/src/NormalizeDataFormatter.php b/src/json-rpc/src/NormalizeDataFormatter.php index b06c198ea..f69f3b54f 100644 --- a/src/json-rpc/src/NormalizeDataFormatter.php +++ b/src/json-rpc/src/NormalizeDataFormatter.php @@ -40,7 +40,7 @@ class NormalizeDataFormatter extends DataFormatter public function formatErrorResponse($data) { - if (isset($data[3]) && $data[3] instanceof \Exception) { + if (isset($data[3]) && $data[3] instanceof \Throwable) { $data[3] = [ 'class' => get_class($data[3]), 'attributes' => $this->normalizer->normalize($data[3]), diff --git a/src/json-rpc/src/ResponseBuilder.php b/src/json-rpc/src/ResponseBuilder.php index 0eed7c6a9..96bce19cd 100644 --- a/src/json-rpc/src/ResponseBuilder.php +++ b/src/json-rpc/src/ResponseBuilder.php @@ -49,7 +49,7 @@ class ResponseBuilder $this->packer = $packer; } - public function buildErrorResponse(ServerRequestInterface $request, int $code, \Exception $error = null): ResponseInterface + public function buildErrorResponse(ServerRequestInterface $request, int $code, \Throwable $error = null): ResponseInterface { $body = new SwooleStream($this->formatErrorResponse($request, $code, $error)); return $this->response()->withAddedHeader('content-type', 'application/json')->withBody($body); @@ -74,7 +74,7 @@ class ResponseBuilder return $this->packer->pack($response); } - protected function formatErrorResponse(ServerRequestInterface $request, int $code, \Exception $error = null): string + protected function formatErrorResponse(ServerRequestInterface $request, int $code, \Throwable $error = null): string { [$code, $message] = $this->error($code, $error ? $error->getMessage() : null); $response = $this->dataFormatter->formatErrorResponse([$request->getAttribute('request_id') ?? '', $code, $message, $error]); diff --git a/src/json-rpc/tests/AnyParamCoreMiddlewareTest.php b/src/json-rpc/tests/AnyParamCoreMiddlewareTest.php index 18d793dba..a79ebdf84 100644 --- a/src/json-rpc/tests/AnyParamCoreMiddlewareTest.php +++ b/src/json-rpc/tests/AnyParamCoreMiddlewareTest.php @@ -147,6 +147,43 @@ class AnyParamCoreMiddlewareTest extends TestCase ], $ret['error']['data']['attributes']); } + public function testThrowable() + { + $container = $this->createContainer(); + $router = $container->make(DispatcherFactory::class, [])->getRouter('jsonrpc'); + $router->addRoute('/CalculatorService/error', [ + CalculatorService::class, 'error', + ]); + $protocol = new Protocol($container, $container->get(ProtocolManager::class), 'jsonrpc'); + $builder = $container->make(ResponseBuilder::class, [ + 'dataFormatter' => $protocol->getDataFormatter(), + 'packer' => $protocol->getPacker(), + ]); + $middleware = new CoreMiddleware($container, $protocol, $builder, 'jsonrpc'); + $handler = \Mockery::mock(RequestHandlerInterface::class); + $request = (new Request('POST', new Uri('/CalculatorService/error'))) + ->withParsedBody([]); + Context::set(ResponseInterface::class, new Response()); + + $request = $middleware->dispatch($request); + try { + $response = $middleware->process($request, $handler); + } catch (\Throwable $exception) { + $response = Context::get(ResponseInterface::class); + } + $this->assertEquals(200, $response->getStatusCode()); + $ret = json_decode((string) $response->getBody(), true); + + $this->assertArrayHasKey('error', $ret); + $this->assertArrayHasKey('data', $ret['error']); + + $this->assertEquals(\Error::class, $ret['error']['data']['class']); + $this->assertArraySubset([ + 'message' => 'Not only a exception.', + 'code' => 0, + ], $ret['error']['data']['attributes']); + } + public function createContainer() { $eventDispatcher = \Mockery::mock(EventDispatcherInterface::class); diff --git a/src/json-rpc/tests/Stub/CalculatorProxyServiceClient.php b/src/json-rpc/tests/Stub/CalculatorProxyServiceClient.php index b01aff1a2..8194a6c89 100644 --- a/src/json-rpc/tests/Stub/CalculatorProxyServiceClient.php +++ b/src/json-rpc/tests/Stub/CalculatorProxyServiceClient.php @@ -35,4 +35,9 @@ class CalculatorProxyServiceClient extends AbstractProxyService implements Calcu { return $this->client->__call(__FUNCTION__, func_get_args()); } + + public function error() + { + return $this->client->__call(__FUNCTION__, func_get_args()); + } } diff --git a/src/json-rpc/tests/Stub/CalculatorService.php b/src/json-rpc/tests/Stub/CalculatorService.php index d469fdd62..bfb8366d0 100644 --- a/src/json-rpc/tests/Stub/CalculatorService.php +++ b/src/json-rpc/tests/Stub/CalculatorService.php @@ -14,17 +14,11 @@ namespace HyperfTest\JsonRpc\Stub; class CalculatorService implements CalculatorServiceInterface { - /** - * {@inheritdoc} - */ public function add(int $a, int $b) { return $a + $b; } - /** - * {@inheritdoc} - */ public function sum(IntegerValue $a, IntegerValue $b): IntegerValue { return IntegerValue::newInstance($a->getValue() + $b->getValue()); @@ -42,4 +36,9 @@ class CalculatorService implements CalculatorServiceInterface { return ['params' => [$a, $b], 'sum' => $a + $b]; } + + public function error() + { + throw new \Error('Not only a exception.'); + } } diff --git a/src/json-rpc/tests/Stub/CalculatorServiceInterface.php b/src/json-rpc/tests/Stub/CalculatorServiceInterface.php index 074ef2601..56fba72c1 100644 --- a/src/json-rpc/tests/Stub/CalculatorServiceInterface.php +++ b/src/json-rpc/tests/Stub/CalculatorServiceInterface.php @@ -21,4 +21,6 @@ interface CalculatorServiceInterface public function divide($value, $divider); public function array(int $a, int $b): array; + + public function error(); } diff --git a/src/rpc-client/src/ServiceClient.php b/src/rpc-client/src/ServiceClient.php index edb7442dc..42545d7c4 100644 --- a/src/rpc-client/src/ServiceClient.php +++ b/src/rpc-client/src/ServiceClient.php @@ -67,7 +67,7 @@ class ServiceClient extends AbstractServiceClient $class = Arr::get($error, 'data.class'); $attributes = Arr::get($error, 'data.attributes', []); if (isset($class) && class_exists($class) && $e = $this->normalizer->denormalize($attributes, $class)) { - if ($e instanceof \Exception) { + if ($e instanceof \Throwable) { throw $e; } } diff --git a/src/utils/src/Serializer/ExceptionNormalizer.php b/src/utils/src/Serializer/ExceptionNormalizer.php index 57f0e53e2..f065ff66a 100644 --- a/src/utils/src/Serializer/ExceptionNormalizer.php +++ b/src/utils/src/Serializer/ExceptionNormalizer.php @@ -32,11 +32,11 @@ class ExceptionNormalizer implements NormalizerInterface, DenormalizerInterface, { if (is_string($data)) { $ex = unserialize($data); - if ($ex instanceof \Exception) { + if ($ex instanceof \Throwable) { return $ex; } - // Retry handle it if the exception not instanceof \Exception. + // Retry handle it if the exception not instanceof \Throwable. $data = $ex; } if (is_array($data) && isset($data['message'], $data['code'])) { @@ -73,7 +73,7 @@ class ExceptionNormalizer implements NormalizerInterface, DenormalizerInterface, */ public function supportsDenormalization($data, $type, $format = null) { - return class_exists($type) && is_a($type, \Exception::class, true); + return class_exists($type) && is_a($type, \Throwable::class, true); } /** @@ -84,7 +84,7 @@ class ExceptionNormalizer implements NormalizerInterface, DenormalizerInterface, if ($object instanceof \Serializable) { return serialize($object); } - /* @var \Exception $object */ + /* @var \Throwable $object */ return [ 'message' => $object->getMessage(), 'code' => $object->getCode(), @@ -98,7 +98,7 @@ class ExceptionNormalizer implements NormalizerInterface, DenormalizerInterface, */ public function supportsNormalization($data, $format = null) { - return $data instanceof \Exception; + return $data instanceof \Throwable; } /** From db2ef0d0bad43d983f2f8fdcc67c4abbe7b7f373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E9=93=AD=E6=98=95?= <715557344@qq.com> Date: Fri, 3 Jan 2020 16:23:34 +0800 Subject: [PATCH 3/9] Added TcpExceptionHandler. --- .../Handler/HttpExceptionHandler.php | 37 +------------ .../Exception/Handler/TcpExceptionHandler.php | 52 +++++++++++++++++++ src/json-rpc/src/TcpServer.php | 4 +- src/json-rpc/tests/TcpServerTest.php | 4 +- 4 files changed, 57 insertions(+), 40 deletions(-) create mode 100644 src/json-rpc/src/Exception/Handler/TcpExceptionHandler.php diff --git a/src/json-rpc/src/Exception/Handler/HttpExceptionHandler.php b/src/json-rpc/src/Exception/Handler/HttpExceptionHandler.php index 52c796ee5..08cefbb98 100644 --- a/src/json-rpc/src/Exception/Handler/HttpExceptionHandler.php +++ b/src/json-rpc/src/Exception/Handler/HttpExceptionHandler.php @@ -12,41 +12,6 @@ declare(strict_types=1); namespace Hyperf\JsonRpc\Exception\Handler; -use Hyperf\Contract\StdoutLoggerInterface; -use Hyperf\ExceptionHandler\ExceptionHandler; -use Hyperf\ExceptionHandler\Formatter\FormatterInterface; -use Psr\Http\Message\ResponseInterface; -use Throwable; - -class HttpExceptionHandler extends ExceptionHandler +class HttpExceptionHandler extends TcpExceptionHandler { - /** - * @var StdoutLoggerInterface - */ - protected $logger; - - /** - * @var FormatterInterface - */ - protected $formatter; - - public function __construct(StdoutLoggerInterface $logger, FormatterInterface $formatter) - { - $this->logger = $logger; - $this->formatter = $formatter; - } - - public function handle(Throwable $throwable, ResponseInterface $response) - { - $this->logger->warning($this->formatter->format($throwable)); - - $this->stopPropagation(); - - return $response; - } - - public function isValid(Throwable $throwable): bool - { - return true; - } } diff --git a/src/json-rpc/src/Exception/Handler/TcpExceptionHandler.php b/src/json-rpc/src/Exception/Handler/TcpExceptionHandler.php new file mode 100644 index 000000000..9c6d695ad --- /dev/null +++ b/src/json-rpc/src/Exception/Handler/TcpExceptionHandler.php @@ -0,0 +1,52 @@ +logger = $logger; + $this->formatter = $formatter; + } + + public function handle(Throwable $throwable, ResponseInterface $response) + { + $this->logger->warning($this->formatter->format($throwable)); + + $this->stopPropagation(); + + return $response; + } + + public function isValid(Throwable $throwable): bool + { + return true; + } +} diff --git a/src/json-rpc/src/TcpServer.php b/src/json-rpc/src/TcpServer.php index 47e2c4fce..3430d0925 100644 --- a/src/json-rpc/src/TcpServer.php +++ b/src/json-rpc/src/TcpServer.php @@ -20,7 +20,7 @@ use Hyperf\HttpMessage\Server\Request as Psr7Request; use Hyperf\HttpMessage\Server\Response as Psr7Response; use Hyperf\HttpMessage\Uri\Uri; use Hyperf\HttpServer\Contract\CoreMiddlewareInterface; -use Hyperf\JsonRpc\Exception\Handler\HttpExceptionHandler; +use Hyperf\JsonRpc\Exception\Handler\TcpExceptionHandler; use Hyperf\Rpc\Protocol; use Hyperf\Rpc\ProtocolManager; use Hyperf\RpcServer\RequestDispatcher; @@ -149,7 +149,7 @@ class TcpServer extends Server protected function getDefaultExceptionHandler(): array { return [ - HttpExceptionHandler::class, + TcpExceptionHandler::class, ]; } } diff --git a/src/json-rpc/tests/TcpServerTest.php b/src/json-rpc/tests/TcpServerTest.php index bbed3cfd9..98e67cfb2 100644 --- a/src/json-rpc/tests/TcpServerTest.php +++ b/src/json-rpc/tests/TcpServerTest.php @@ -16,7 +16,7 @@ use Hyperf\Config\Config; use Hyperf\Contract\ContainerInterface; use Hyperf\Contract\StdoutLoggerInterface; use Hyperf\ExceptionHandler\ExceptionHandlerDispatcher; -use Hyperf\JsonRpc\Exception\Handler\HttpExceptionHandler; +use Hyperf\JsonRpc\Exception\Handler\TcpExceptionHandler; use Hyperf\JsonRpc\TcpServer; use Hyperf\Rpc\ProtocolManager; use Hyperf\RpcServer\RequestDispatcher; @@ -46,7 +46,7 @@ class TcpServerTest extends TestCase $method->setAccessible(true); $res = $method->invoke($server); - $this->assertSame([HttpExceptionHandler::class], $res); + $this->assertSame([TcpExceptionHandler::class], $res); } protected function getContainer() From 656032d009077e12695b46e81decc468bc12cf68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E9=93=AD=E6=98=95?= <715557344@qq.com> Date: Mon, 6 Jan 2020 15:23:17 +0800 Subject: [PATCH 4/9] Added data of throwable into RequestException. --- src/json-rpc/src/DataFormatter.php | 1 + src/json-rpc/tests/DataFormatterTest.php | 90 +++++++++++++++++++ .../src/Exception/RequestException.php | 43 +++++++++ src/rpc-client/src/ServiceClient.php | 2 +- 4 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 src/json-rpc/tests/DataFormatterTest.php diff --git a/src/json-rpc/src/DataFormatter.php b/src/json-rpc/src/DataFormatter.php index e4c96fda5..72714b3d7 100644 --- a/src/json-rpc/src/DataFormatter.php +++ b/src/json-rpc/src/DataFormatter.php @@ -44,6 +44,7 @@ class DataFormatter implements DataFormatterInterface if (isset($data) && $data instanceof \Throwable) { $data = [ 'class' => get_class($data), + 'code' => $data->getCode(), 'message' => $data->getMessage(), ]; } diff --git a/src/json-rpc/tests/DataFormatterTest.php b/src/json-rpc/tests/DataFormatterTest.php new file mode 100644 index 000000000..fc6494845 --- /dev/null +++ b/src/json-rpc/tests/DataFormatterTest.php @@ -0,0 +1,90 @@ +formatErrorResponse([$id = uniqid(), 500, 'Error', new \RuntimeException('test case', 1000)]); + + $this->assertEquals([ + 'jsonrpc' => '2.0', + 'id' => $id, + 'error' => [ + 'code' => 500, + 'message' => 'Error', + 'data' => [ + 'class' => 'RuntimeException', + 'code' => 1000, + 'message' => 'test case', + ], + ], + ], $data); + + $exception = new RequestException('', 0, $data['error']['data']); + $this->assertSame(1000, $exception->getThrowableCode()); + $this->assertSame('test case', $exception->getThrowableMessage()); + } + + public function testNormalizeFormatErrorResponse() + { + $normalizer = new SymfonyNormalizer((new SerializerFactory())()); + + $formatter = new NormalizeDataFormatter($normalizer); + $data = $formatter->formatErrorResponse([$id = uniqid(), 500, 'Error', new \RuntimeException('test case', 1000)]); + + $this->assertArrayHasKey('line', $data['error']['data']['attributes']); + $this->assertArrayHasKey('file', $data['error']['data']['attributes']); + + $exception = new RequestException('', 0, $data['error']['data']); + $this->assertSame(1000, $exception->getThrowableCode()); + $this->assertSame('test case', $exception->getThrowableMessage()); + + unset($data['error']['data']['attributes']['line'], $data['error']['data']['attributes']['file']); + + $this->assertEquals([ + 'jsonrpc' => '2.0', + 'id' => $id, + 'error' => [ + 'code' => 500, + 'message' => 'Error', + 'data' => [ + 'class' => 'RuntimeException', + 'attributes' => [ + 'code' => 1000, + 'message' => 'test case', + ], + ], + ], + ], $data); + } +} diff --git a/src/rpc-client/src/Exception/RequestException.php b/src/rpc-client/src/Exception/RequestException.php index c21b707ef..3a8aefcaf 100644 --- a/src/rpc-client/src/Exception/RequestException.php +++ b/src/rpc-client/src/Exception/RequestException.php @@ -14,4 +14,47 @@ namespace Hyperf\RpcClient\Exception; class RequestException extends \RuntimeException { + protected $throwable; + + /** + * @param $throwable = [ + * 'class' => 'RuntimeException', // The exception class + * 'code' => 0, // The exception code + * 'message' => '', // The exception message + * 'attributes' => [ + * 'message' => '', // The exception message + * 'code' => 0, // The exception code + * 'file' => '/opt/www/hyperf/app/JsonRpc/CalculatorService.php', // The file in which the exception occurred + * 'line' => 99, // The line in which the exception occurred + * ], + * ] + * @param string $message + * @param int $code + */ + public function __construct($message = '', $code = 0, array $throwable) + { + parent::__construct($message, $code); + + $this->throwable = $throwable; + } + + public function getThrowable(): array + { + return $this->throwable; + } + + public function getThrowableCode(): int + { + return intval($this->throwable['code'] ?? $this->throwable['attributes']['code'] ?? 0); + } + + public function getThrowableMessage(): string + { + return strval($this->throwable['message'] ?? $this->throwable['attributes']['message'] ?? ''); + } + + public function getThrowableClassName(): string + { + return strval($this->throwable['class']); + } } diff --git a/src/rpc-client/src/ServiceClient.php b/src/rpc-client/src/ServiceClient.php index 42545d7c4..f595551e9 100644 --- a/src/rpc-client/src/ServiceClient.php +++ b/src/rpc-client/src/ServiceClient.php @@ -73,7 +73,7 @@ class ServiceClient extends AbstractServiceClient } // Throw RequestException when denormalize exception failed. - throw new RequestException($error['message'] ?? '', $error['code']); + throw new RequestException($error['message'] ?? '', $code, $error['data'] ?? []); } throw new RequestException('Invalid response.'); From 5f0c6e383d24f0178d1c6ee4d518701df31db0aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E9=93=AD=E6=98=95?= <715557344@qq.com> Date: Mon, 6 Jan 2020 16:05:11 +0800 Subject: [PATCH 5/9] Request id must is equal to response id. --- src/json-rpc/tests/RpcServiceClientTest.php | 66 +++++++++++++++---- src/rpc-client/src/AbstractServiceClient.php | 11 ++-- src/rpc-client/src/ConfigProvider.php | 5 ++ .../src/Exception/RequestException.php | 5 +- .../src/IdGenerator/IdGeneratorInterface.php | 19 ++++++ .../src/IdGenerator/UniqidIdGenerator.php | 21 ++++++ src/rpc-client/src/ServiceClient.php | 4 ++ 7 files changed, 114 insertions(+), 17 deletions(-) create mode 100644 src/rpc-client/src/IdGenerator/IdGeneratorInterface.php create mode 100644 src/rpc-client/src/IdGenerator/UniqidIdGenerator.php diff --git a/src/json-rpc/tests/RpcServiceClientTest.php b/src/json-rpc/tests/RpcServiceClientTest.php index e37186a9c..56d7b10c4 100644 --- a/src/json-rpc/tests/RpcServiceClientTest.php +++ b/src/json-rpc/tests/RpcServiceClientTest.php @@ -26,6 +26,9 @@ use Hyperf\JsonRpc\JsonRpcTransporter; use Hyperf\JsonRpc\NormalizeDataFormatter; use Hyperf\JsonRpc\PathGenerator; use Hyperf\Logger\Logger; +use Hyperf\RpcClient\Exception\RequestException; +use Hyperf\RpcClient\IdGenerator\IdGeneratorInterface; +use Hyperf\RpcClient\IdGenerator\UniqidIdGenerator; use Hyperf\RpcClient\ProxyFactory; use Hyperf\Utils\ApplicationContext; use Hyperf\Utils\Packer\JsonPacker; @@ -59,9 +62,13 @@ class RpcServiceClientTest extends TestCase $transporter->shouldReceive('setLoadBalancer') ->andReturnSelf(); $transporter->shouldReceive('send') - ->andReturn(json_encode([ - 'result' => 3, - ])); + ->andReturnUsing(function ($data) { + $id = json_decode($data, true)['id']; + return json_encode([ + 'id' => $id, + 'result' => 3, + ]); + }); $service = new CalculatorProxyServiceClient($container, CalculatorServiceInterface::class, 'jsonrpc'); $ret = $service->add(1, 2); $this->assertEquals(3, $ret); @@ -76,9 +83,13 @@ class RpcServiceClientTest extends TestCase $transporter->shouldReceive('setLoadBalancer') ->andReturnSelf(); $transporter->shouldReceive('send') - ->andReturn(json_encode([ - 'result' => ['params' => [1, 2], 'sum' => 3], - ])); + ->andReturnUsing(function ($data) { + $id = json_decode($data, true)['id']; + return json_encode([ + 'id' => $id, + 'result' => ['params' => [1, 2], 'sum' => 3], + ]); + }); $service = new CalculatorProxyServiceClient($container, CalculatorServiceInterface::class, 'jsonrpc'); $ret = $service->array(1, 2); $this->assertEquals(['params' => [1, 2], 'sum' => 3], $ret); @@ -92,9 +103,13 @@ class RpcServiceClientTest extends TestCase $transporter->shouldReceive('setLoadBalancer') ->andReturnSelf(); $transporter->shouldReceive('send') - ->andReturn(json_encode([ - 'result' => 3, - ])); + ->andReturnUsing(function ($data) { + $id = json_decode($data, true)['id']; + return json_encode([ + 'id' => $id, + 'result' => 3, + ]); + }); $factory = new ProxyFactory(); $proxyClass = $factory->createProxy(CalculatorServiceInterface::class); /** @var CalculatorServiceInterface $service */ @@ -103,6 +118,28 @@ class RpcServiceClientTest extends TestCase $this->assertEquals(3, $ret); } + public function testProxyFactoryWithErrorId() + { + $container = $this->createContainer(); + /** @var MockInterface $transporter */ + $transporter = $container->get(JsonRpcTransporter::class); + $transporter->shouldReceive('setLoadBalancer') + ->andReturnSelf(); + $transporter->shouldReceive('send') + ->andReturn(json_encode([ + 'id' => '1234', + 'result' => 3, + ])); + $factory = new ProxyFactory(); + $proxyClass = $factory->createProxy(CalculatorServiceInterface::class); + /** @var CalculatorServiceInterface $service */ + $service = new $proxyClass($container, CalculatorServiceInterface::class, 'jsonrpc'); + + $this->expectException(RequestException::class); + $this->expectExceptionMessageRegExp('/^Invalid response\. Request id\[.*\] is not equal to response id\[1234\]\.$/'); + $service->add(1, 2); + } + public function testProxyFactoryObjectParameter() { $container = $this->createContainer(); @@ -111,9 +148,13 @@ class RpcServiceClientTest extends TestCase $transporter->shouldReceive('setLoadBalancer') ->andReturnSelf(); $transporter->shouldReceive('send') - ->andReturn(json_encode([ - 'result' => ['value' => 3], - ])); + ->andReturnUsing(function ($data) { + $id = json_decode($data, true)['id']; + return json_encode([ + 'id' => $id, + 'result' => ['value' => 3], + ]); + }); $factory = new ProxyFactory(); $proxyClass = $factory->createProxy(CalculatorServiceInterface::class); /** @var CalculatorServiceInterface $service */ @@ -159,6 +200,7 @@ class RpcServiceClientTest extends TestCase JsonRpcTransporter::class => function () use ($transporter) { return $transporter; }, + IdGeneratorInterface::class => UniqidIdGenerator::class, ], new ScanConfig())); ApplicationContext::setContainer($container); return $container; diff --git a/src/rpc-client/src/AbstractServiceClient.php b/src/rpc-client/src/AbstractServiceClient.php index ee18a54ba..9ba110f04 100644 --- a/src/rpc-client/src/AbstractServiceClient.php +++ b/src/rpc-client/src/AbstractServiceClient.php @@ -24,6 +24,7 @@ use Hyperf\Rpc\Contract\DataFormatterInterface; use Hyperf\Rpc\Contract\PathGeneratorInterface; use Hyperf\Rpc\Protocol; use Hyperf\Rpc\ProtocolManager; +use Hyperf\RpcClient\Exception\RequestException; use InvalidArgumentException; use Psr\Container\ContainerInterface; use RuntimeException; @@ -93,9 +94,7 @@ abstract class AbstractServiceClient $this->client = make(Client::class) ->setPacker($protocol->getPacker()) ->setTransporter($transporter); - if ($container->has(IdGeneratorInterface::class)) { - $this->idGenerator = $container->get(IdGeneratorInterface::class); - } + $this->idGenerator = $container->get(IdGenerator\IdGeneratorInterface::class); $this->pathGenerator = $protocol->getPathGenerator(); $this->dataFormatter = $protocol->getDataFormatter(); } @@ -107,6 +106,10 @@ abstract class AbstractServiceClient } $response = $this->client->send($this->__generateData($method, $params, $id)); if (is_array($response)) { + if (! isset($response['id']) || $response['id'] !== $id) { + throw new RequestException(sprintf('Invalid response. Request id[%s] is not equal to response id[%s].', $id, $response['id'] ?? null)); + } + if (array_key_exists('result', $response)) { return $response['result']; } @@ -114,7 +117,7 @@ abstract class AbstractServiceClient return $response['error']; } } - throw new RuntimeException('Invalid response.'); + throw new RequestException('Invalid response.'); } protected function __generateRpcPath(string $methodName): string diff --git a/src/rpc-client/src/ConfigProvider.php b/src/rpc-client/src/ConfigProvider.php index e341c93c5..00d1214b4 100644 --- a/src/rpc-client/src/ConfigProvider.php +++ b/src/rpc-client/src/ConfigProvider.php @@ -12,6 +12,8 @@ declare(strict_types=1); namespace Hyperf\RpcClient; +use Hyperf\RpcClient\IdGenerator\IdGeneratorInterface; +use Hyperf\RpcClient\IdGenerator\UniqidIdGenerator; use Hyperf\RpcClient\Listener\AddConsumerDefinitionListener; class ConfigProvider @@ -22,6 +24,9 @@ class ConfigProvider 'listeners' => [ AddConsumerDefinitionListener::class, ], + 'dependencies' => [ + IdGeneratorInterface::class => UniqidIdGenerator::class, + ], 'annotations' => [ 'scan' => [ 'paths' => [ diff --git a/src/rpc-client/src/Exception/RequestException.php b/src/rpc-client/src/Exception/RequestException.php index 3a8aefcaf..fa483993c 100644 --- a/src/rpc-client/src/Exception/RequestException.php +++ b/src/rpc-client/src/Exception/RequestException.php @@ -14,6 +14,9 @@ namespace Hyperf\RpcClient\Exception; class RequestException extends \RuntimeException { + /** + * @var array + */ protected $throwable; /** @@ -31,7 +34,7 @@ class RequestException extends \RuntimeException * @param string $message * @param int $code */ - public function __construct($message = '', $code = 0, array $throwable) + public function __construct($message = '', $code = 0, array $throwable = []) { parent::__construct($message, $code); diff --git a/src/rpc-client/src/IdGenerator/IdGeneratorInterface.php b/src/rpc-client/src/IdGenerator/IdGeneratorInterface.php new file mode 100644 index 000000000..76d477311 --- /dev/null +++ b/src/rpc-client/src/IdGenerator/IdGeneratorInterface.php @@ -0,0 +1,19 @@ +methodDefinitionCollector->getReturnType($this->serviceInterface, $method); return $this->normalizer->denormalize($response['result'], $type->getName()); From 552b5f2a3fac3dfbd7250f57e332eaff32b09799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E9=93=AD=E6=98=95?= <715557344@qq.com> Date: Mon, 6 Jan 2020 18:49:25 +0800 Subject: [PATCH 6/9] Added method `recv` into TransporterInterface. --- src/json-rpc/src/JsonRpcHttpTransporter.php | 5 +++ src/json-rpc/src/JsonRpcPoolTransporter.php | 44 +++++++++++++------ src/json-rpc/src/JsonRpcTransporter.php | 15 ++++++- src/json-rpc/src/Pool/RpcConnection.php | 1 + src/json-rpc/tests/RpcServiceClientTest.php | 4 ++ src/rpc-client/src/AbstractServiceClient.php | 24 ++++++++-- src/rpc-client/src/Client.php | 7 +++ src/rpc-client/src/ServiceClient.php | 4 +- src/rpc/src/Contract/TransporterInterface.php | 2 + 9 files changed, 86 insertions(+), 20 deletions(-) diff --git a/src/json-rpc/src/JsonRpcHttpTransporter.php b/src/json-rpc/src/JsonRpcHttpTransporter.php index 3b0e93231..0a73930e7 100644 --- a/src/json-rpc/src/JsonRpcHttpTransporter.php +++ b/src/json-rpc/src/JsonRpcHttpTransporter.php @@ -84,6 +84,11 @@ class JsonRpcHttpTransporter implements TransporterInterface return ''; } + public function recv() + { + throw new \RuntimeException('JsonRpcHttpTransporter not support recv.'); + } + public function getClient(): Client { return $this->clientFactory->create([ diff --git a/src/json-rpc/src/JsonRpcPoolTransporter.php b/src/json-rpc/src/JsonRpcPoolTransporter.php index b47b197f9..b975342f2 100644 --- a/src/json-rpc/src/JsonRpcPoolTransporter.php +++ b/src/json-rpc/src/JsonRpcPoolTransporter.php @@ -12,12 +12,14 @@ declare(strict_types=1); namespace Hyperf\JsonRpc; +use Hyperf\Contract\ConnectionInterface; use Hyperf\JsonRpc\Pool\PoolFactory; use Hyperf\JsonRpc\Pool\RpcConnection; use Hyperf\LoadBalancer\LoadBalancerInterface; use Hyperf\LoadBalancer\Node; use Hyperf\Pool\Pool; use Hyperf\Rpc\Contract\TransporterInterface; +use Hyperf\Utils\Context; use RuntimeException; class JsonRpcPoolTransporter implements TransporterInterface @@ -76,11 +78,8 @@ class JsonRpcPoolTransporter implements TransporterInterface public function send(string $data) { $client = retry(2, function () use ($data) { - $pool = $this->getPool(); - $connection = $pool->get(); try { - /** @var RpcConnection $client */ - $client = $connection->getConnection(); + $client = $this->getConnection(); if ($client->send($data) === false) { if ($client->errCode == 104) { throw new RuntimeException('Connect to server failed.'); @@ -88,20 +87,39 @@ class JsonRpcPoolTransporter implements TransporterInterface } return $client; } catch (\Throwable $throwable) { - if ($connection instanceof RpcConnection) { - // Reconnect again next time. - $connection->resetLastUseTime(); + if (isset($client) && $client instanceof ConnectionInterface) { + $client->close(); } - $connection->release(); throw $throwable; } }); - try { - $data = $client->recv($this->recvTimeout); - } finally { - $client->release(); + + return $client->recv($this->recvTimeout); + } + + public function recv() + { + return $this->getConnection()->recv($this->recvTimeout); + } + + /** + * Get RpcConnection from Context. + */ + public function getConnection(): RpcConnection + { + $class = static::class . '.Connection'; + if (Context::has($class)) { + return Context::get($class); } - return $data; + + $pool = $this->getPool(); + $connection = $pool->get(); + + defer(function () use ($connection) { + $connection->release(); + }); + + return Context::set($class, $connection->getConnection()); } public function getPool(): Pool diff --git a/src/json-rpc/src/JsonRpcTransporter.php b/src/json-rpc/src/JsonRpcTransporter.php index 07058931e..4af2a6d9a 100644 --- a/src/json-rpc/src/JsonRpcTransporter.php +++ b/src/json-rpc/src/JsonRpcTransporter.php @@ -15,6 +15,7 @@ namespace Hyperf\JsonRpc; use Hyperf\LoadBalancer\LoadBalancerInterface; use Hyperf\LoadBalancer\Node; use Hyperf\Rpc\Contract\TransporterInterface; +use Hyperf\Utils\Context; use RuntimeException; use Swoole\Coroutine\Client as SwooleClient; @@ -70,12 +71,22 @@ class JsonRpcTransporter implements TransporterInterface return $client->recv($this->recvTimeout); } + public function recv() + { + return $this->getClient()->recv($this->recvTimeout); + } + public function getClient(): SwooleClient { + $class = static::class . '.Connection'; + if (Context::has($class)) { + return Context::get($class); + } + $client = new SwooleClient(SWOOLE_SOCK_TCP); $client->set($this->config['settings'] ?? []); - return retry(2, function () use ($client) { + $client = retry(2, function () use ($client) { $node = $this->getNode(); $result = $client->connect($node->host, $node->port, $this->connectTimeout); if ($result === false && ($client->errCode == 114 or $client->errCode == 115)) { @@ -85,6 +96,8 @@ class JsonRpcTransporter implements TransporterInterface } return $client; }); + + return Context::set($class, $client); } public function getLoadBalancer(): ?LoadBalancerInterface diff --git a/src/json-rpc/src/Pool/RpcConnection.php b/src/json-rpc/src/Pool/RpcConnection.php index c88d70c03..ed1b911ea 100644 --- a/src/json-rpc/src/Pool/RpcConnection.php +++ b/src/json-rpc/src/Pool/RpcConnection.php @@ -97,6 +97,7 @@ class RpcConnection extends BaseConnection implements ConnectionInterface public function close(): bool { + $this->lastUseTime = 0.0; $this->connection->close(); return true; } diff --git a/src/json-rpc/tests/RpcServiceClientTest.php b/src/json-rpc/tests/RpcServiceClientTest.php index 56d7b10c4..a7d55ace2 100644 --- a/src/json-rpc/tests/RpcServiceClientTest.php +++ b/src/json-rpc/tests/RpcServiceClientTest.php @@ -130,6 +130,10 @@ class RpcServiceClientTest extends TestCase 'id' => '1234', 'result' => 3, ])); + $transporter->shouldReceive('recv')->andReturn(json_encode([ + 'id' => '1234', + 'result' => 3, + ])); $factory = new ProxyFactory(); $proxyClass = $factory->createProxy(CalculatorServiceInterface::class); /** @var CalculatorServiceInterface $service */ diff --git a/src/rpc-client/src/AbstractServiceClient.php b/src/rpc-client/src/AbstractServiceClient.php index 9ba110f04..89c20b692 100644 --- a/src/rpc-client/src/AbstractServiceClient.php +++ b/src/rpc-client/src/AbstractServiceClient.php @@ -106,9 +106,7 @@ abstract class AbstractServiceClient } $response = $this->client->send($this->__generateData($method, $params, $id)); if (is_array($response)) { - if (! isset($response['id']) || $response['id'] !== $id) { - throw new RequestException(sprintf('Invalid response. Request id[%s] is not equal to response id[%s].', $id, $response['id'] ?? null)); - } + $response = $this->checkRequestIdAndTryAgain($response, $id); if (array_key_exists('result', $response)) { return $response['result']; @@ -253,4 +251,24 @@ abstract class AbstractServiceClient }, ]); } + + protected function checkRequestIdAndTryAgain(array $response, $id, int $again = 1): array + { + if (isset($response['id']) && $response['id'] === $id) { + return $response; + } + + $response = $this->client->recv(); + if ($again <= 0) { + throw new RequestException(sprintf( + 'Invalid response. Request id[%s] is not equal to response id[%s].', + $id, + $response['id'] ?? null + )); + } + + --$again; + + return $this->checkRequestIdAndTryAgain($response, $id, $again); + } } diff --git a/src/rpc-client/src/Client.php b/src/rpc-client/src/Client.php index ee809c94e..44ee02102 100644 --- a/src/rpc-client/src/Client.php +++ b/src/rpc-client/src/Client.php @@ -42,6 +42,13 @@ class Client return $packer->unpack((string) $response); } + public function recv() + { + $packer = $this->getPacker(); + $response = $this->getTransporter()->recv(); + return $packer->unpack((string) $response); + } + public function getPacker(): PackerInterface { return $this->packer; diff --git a/src/rpc-client/src/ServiceClient.php b/src/rpc-client/src/ServiceClient.php index bfc03ec6e..5e6d9141c 100644 --- a/src/rpc-client/src/ServiceClient.php +++ b/src/rpc-client/src/ServiceClient.php @@ -56,9 +56,7 @@ class ServiceClient extends AbstractServiceClient throw new RequestException('Invalid response.'); } - if (! isset($response['id']) || $response['id'] !== $id) { - throw new RequestException(sprintf('Invalid response. Request id[%s] is not equal to response id[%s].', $id, $response['id'] ?? null)); - } + $response = $this->checkRequestIdAndTryAgain($response, $id); if (isset($response['result'])) { $type = $this->methodDefinitionCollector->getReturnType($this->serviceInterface, $method); diff --git a/src/rpc/src/Contract/TransporterInterface.php b/src/rpc/src/Contract/TransporterInterface.php index 75ba9180c..a74262965 100644 --- a/src/rpc/src/Contract/TransporterInterface.php +++ b/src/rpc/src/Contract/TransporterInterface.php @@ -18,6 +18,8 @@ interface TransporterInterface { public function send(string $data); + public function recv(); + public function getLoadBalancer(): ?LoadBalancerInterface; public function setLoadBalancer(LoadBalancerInterface $loadBalancer): TransporterInterface; From cf59aca5d638977775439b7d5d54d841543f5a13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E9=93=AD=E6=98=95?= <715557344@qq.com> Date: Tue, 7 Jan 2020 10:49:49 +0800 Subject: [PATCH 7/9] Throw exception when recv failed. --- src/json-rpc/src/JsonRpcPoolTransporter.php | 8 +++- src/json-rpc/src/JsonRpcTransporter.php | 9 ++++- src/json-rpc/src/RecvTrait.php | 37 +++++++++++++++++++ src/json-rpc/tests/RpcServiceClientTest.php | 4 +- src/rpc-client/src/AbstractServiceClient.php | 1 + src/rpc-client/src/ConfigProvider.php | 4 +- src/rpc/src/Exception/RecvException.php | 17 +++++++++ .../src/IdGenerator/IdGeneratorInterface.php | 2 +- .../src/IdGenerator/UniqidIdGenerator.php | 2 +- 9 files changed, 74 insertions(+), 10 deletions(-) create mode 100644 src/json-rpc/src/RecvTrait.php create mode 100644 src/rpc/src/Exception/RecvException.php rename src/{rpc-client => rpc}/src/IdGenerator/IdGeneratorInterface.php (89%) rename src/{rpc-client => rpc}/src/IdGenerator/UniqidIdGenerator.php (90%) diff --git a/src/json-rpc/src/JsonRpcPoolTransporter.php b/src/json-rpc/src/JsonRpcPoolTransporter.php index b975342f2..cde3e23a2 100644 --- a/src/json-rpc/src/JsonRpcPoolTransporter.php +++ b/src/json-rpc/src/JsonRpcPoolTransporter.php @@ -24,6 +24,8 @@ use RuntimeException; class JsonRpcPoolTransporter implements TransporterInterface { + use RecvTrait; + /** * @var PoolFactory */ @@ -94,12 +96,14 @@ class JsonRpcPoolTransporter implements TransporterInterface } }); - return $client->recv($this->recvTimeout); + return $this->recvAndCheck($client, $this->recvTimeout); } public function recv() { - return $this->getConnection()->recv($this->recvTimeout); + $client = $this->getConnection(); + + return $this->recvAndCheck($client, $this->recvTimeout); } /** diff --git a/src/json-rpc/src/JsonRpcTransporter.php b/src/json-rpc/src/JsonRpcTransporter.php index 4af2a6d9a..3b4523062 100644 --- a/src/json-rpc/src/JsonRpcTransporter.php +++ b/src/json-rpc/src/JsonRpcTransporter.php @@ -21,6 +21,8 @@ use Swoole\Coroutine\Client as SwooleClient; class JsonRpcTransporter implements TransporterInterface { + use RecvTrait; + /** * @var null|LoadBalancerInterface */ @@ -68,12 +70,15 @@ class JsonRpcTransporter implements TransporterInterface } return $client; }); - return $client->recv($this->recvTimeout); + + return $this->recvAndCheck($client, $this->recvTimeout); } public function recv() { - return $this->getClient()->recv($this->recvTimeout); + $client = $this->getClient(); + + return $this->recvAndCheck($client, $this->recvTimeout); } public function getClient(): SwooleClient diff --git a/src/json-rpc/src/RecvTrait.php b/src/json-rpc/src/RecvTrait.php new file mode 100644 index 000000000..30c8ffb2d --- /dev/null +++ b/src/json-rpc/src/RecvTrait.php @@ -0,0 +1,37 @@ +recv((float) $timeout); + if ($data === '') { + throw new RecvException('Connection is closed.'); + } + if ($data === false) { + throw new RecvException('Error receiving data, errno=' . $client->errCode); + } + + return $data; + } +} diff --git a/src/json-rpc/tests/RpcServiceClientTest.php b/src/json-rpc/tests/RpcServiceClientTest.php index a7d55ace2..b67078e94 100644 --- a/src/json-rpc/tests/RpcServiceClientTest.php +++ b/src/json-rpc/tests/RpcServiceClientTest.php @@ -26,9 +26,9 @@ use Hyperf\JsonRpc\JsonRpcTransporter; use Hyperf\JsonRpc\NormalizeDataFormatter; use Hyperf\JsonRpc\PathGenerator; use Hyperf\Logger\Logger; +use Hyperf\Rpc\IdGenerator\IdGeneratorInterface; +use Hyperf\Rpc\IdGenerator\UniqidIdGenerator; use Hyperf\RpcClient\Exception\RequestException; -use Hyperf\RpcClient\IdGenerator\IdGeneratorInterface; -use Hyperf\RpcClient\IdGenerator\UniqidIdGenerator; use Hyperf\RpcClient\ProxyFactory; use Hyperf\Utils\ApplicationContext; use Hyperf\Utils\Packer\JsonPacker; diff --git a/src/rpc-client/src/AbstractServiceClient.php b/src/rpc-client/src/AbstractServiceClient.php index 89c20b692..a6ea27db4 100644 --- a/src/rpc-client/src/AbstractServiceClient.php +++ b/src/rpc-client/src/AbstractServiceClient.php @@ -22,6 +22,7 @@ use Hyperf\LoadBalancer\LoadBalancerManager; use Hyperf\LoadBalancer\Node; use Hyperf\Rpc\Contract\DataFormatterInterface; use Hyperf\Rpc\Contract\PathGeneratorInterface; +use Hyperf\Rpc\IdGenerator; use Hyperf\Rpc\Protocol; use Hyperf\Rpc\ProtocolManager; use Hyperf\RpcClient\Exception\RequestException; diff --git a/src/rpc-client/src/ConfigProvider.php b/src/rpc-client/src/ConfigProvider.php index 00d1214b4..723e6cb34 100644 --- a/src/rpc-client/src/ConfigProvider.php +++ b/src/rpc-client/src/ConfigProvider.php @@ -12,8 +12,8 @@ declare(strict_types=1); namespace Hyperf\RpcClient; -use Hyperf\RpcClient\IdGenerator\IdGeneratorInterface; -use Hyperf\RpcClient\IdGenerator\UniqidIdGenerator; +use Hyperf\Rpc\IdGenerator\IdGeneratorInterface; +use Hyperf\Rpc\IdGenerator\UniqidIdGenerator; use Hyperf\RpcClient\Listener\AddConsumerDefinitionListener; class ConfigProvider diff --git a/src/rpc/src/Exception/RecvException.php b/src/rpc/src/Exception/RecvException.php new file mode 100644 index 000000000..76f6a730b --- /dev/null +++ b/src/rpc/src/Exception/RecvException.php @@ -0,0 +1,17 @@ + Date: Tue, 7 Jan 2020 18:21:54 +0800 Subject: [PATCH 8/9] Optimized code. --- src/json-rpc/src/JsonRpcPoolTransporter.php | 3 +-- .../src/Listener/RegisterServiceListener.php | 2 +- src/json-rpc/src/RecvTrait.php | 3 +++ src/json-rpc/tests/RpcServiceClientTest.php | 13 +++++++++---- src/rpc-client/src/AbstractServiceClient.php | 17 +++++++++++++++-- src/rpc-client/src/ConfigProvider.php | 5 ----- 6 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/json-rpc/src/JsonRpcPoolTransporter.php b/src/json-rpc/src/JsonRpcPoolTransporter.php index cde3e23a2..2c5cb29df 100644 --- a/src/json-rpc/src/JsonRpcPoolTransporter.php +++ b/src/json-rpc/src/JsonRpcPoolTransporter.php @@ -116,8 +116,7 @@ class JsonRpcPoolTransporter implements TransporterInterface return Context::get($class); } - $pool = $this->getPool(); - $connection = $pool->get(); + $connection = $this->getPool()->get(); defer(function () use ($connection) { $connection->release(); diff --git a/src/json-rpc/src/Listener/RegisterServiceListener.php b/src/json-rpc/src/Listener/RegisterServiceListener.php index 058323ab1..e392a0b5b 100644 --- a/src/json-rpc/src/Listener/RegisterServiceListener.php +++ b/src/json-rpc/src/Listener/RegisterServiceListener.php @@ -44,7 +44,7 @@ class RegisterServiceListener implements ListenerInterface public function process(object $event) { $annotation = $event->annotation; - if (! in_array($annotation->protocol, ['jsonrpc', 'jsonrpc-http'])) { + if (! in_array($annotation->protocol, ['jsonrpc', 'jsonrpc-http', 'jsonrpc-tcp-length-check'])) { return; } $metadata = $event->toArray(); diff --git a/src/json-rpc/src/RecvTrait.php b/src/json-rpc/src/RecvTrait.php index 30c8ffb2d..e28b51090 100644 --- a/src/json-rpc/src/RecvTrait.php +++ b/src/json-rpc/src/RecvTrait.php @@ -26,6 +26,9 @@ trait RecvTrait { $data = $client->recv((float) $timeout); if ($data === '') { + // RpcConnection: When the next time the connection is taken out of the connection pool, it will reconnecting to the target service. + // Client: It will reconnecting to the target service in the next request. + $client->close(); throw new RecvException('Connection is closed.'); } if ($data === false) { diff --git a/src/json-rpc/tests/RpcServiceClientTest.php b/src/json-rpc/tests/RpcServiceClientTest.php index b67078e94..54b43fdde 100644 --- a/src/json-rpc/tests/RpcServiceClientTest.php +++ b/src/json-rpc/tests/RpcServiceClientTest.php @@ -130,10 +130,15 @@ class RpcServiceClientTest extends TestCase 'id' => '1234', 'result' => 3, ])); - $transporter->shouldReceive('recv')->andReturn(json_encode([ - 'id' => '1234', - 'result' => 3, - ])); + $once = true; + $transporter->shouldReceive('recv')->andReturnUsing(function () use (&$once) { + $this->assertTrue($once); + $once = false; + return json_encode([ + 'id' => '1234', + 'result' => 3, + ]); + }); $factory = new ProxyFactory(); $proxyClass = $factory->createProxy(CalculatorServiceInterface::class); /** @var CalculatorServiceInterface $service */ diff --git a/src/rpc-client/src/AbstractServiceClient.php b/src/rpc-client/src/AbstractServiceClient.php index a6ea27db4..24fb41ba6 100644 --- a/src/rpc-client/src/AbstractServiceClient.php +++ b/src/rpc-client/src/AbstractServiceClient.php @@ -95,7 +95,7 @@ abstract class AbstractServiceClient $this->client = make(Client::class) ->setPacker($protocol->getPacker()) ->setTransporter($transporter); - $this->idGenerator = $container->get(IdGenerator\IdGeneratorInterface::class); + $this->idGenerator = $this->getIdGenerator(); $this->pathGenerator = $protocol->getPathGenerator(); $this->dataFormatter = $protocol->getDataFormatter(); } @@ -132,6 +132,19 @@ abstract class AbstractServiceClient return $this->dataFormatter->formatRequest([$this->__generateRpcPath($methodName), $params, $id]); } + protected function getIdGenerator(): IdGeneratorInterface + { + if ($this->container->has(IdGenerator\IdGeneratorInterface::class)) { + return $this->container->get(IdGenerator\IdGeneratorInterface::class); + } + + if ($this->container->has(IdGeneratorInterface::class)) { + return $this->container->get(IdGeneratorInterface::class); + } + + return $this->container->get(IdGenerator\UniqidIdGenerator::class); + } + protected function createLoadBalancer(array $nodes, callable $refresh = null): LoadBalancerInterface { $loadBalancer = $this->loadBalancerManager->getInstance($this->serviceName, $this->loadBalancer)->setNodes($nodes); @@ -259,7 +272,6 @@ abstract class AbstractServiceClient return $response; } - $response = $this->client->recv(); if ($again <= 0) { throw new RequestException(sprintf( 'Invalid response. Request id[%s] is not equal to response id[%s].', @@ -268,6 +280,7 @@ abstract class AbstractServiceClient )); } + $response = $this->client->recv(); --$again; return $this->checkRequestIdAndTryAgain($response, $id, $again); diff --git a/src/rpc-client/src/ConfigProvider.php b/src/rpc-client/src/ConfigProvider.php index 723e6cb34..e341c93c5 100644 --- a/src/rpc-client/src/ConfigProvider.php +++ b/src/rpc-client/src/ConfigProvider.php @@ -12,8 +12,6 @@ declare(strict_types=1); namespace Hyperf\RpcClient; -use Hyperf\Rpc\IdGenerator\IdGeneratorInterface; -use Hyperf\Rpc\IdGenerator\UniqidIdGenerator; use Hyperf\RpcClient\Listener\AddConsumerDefinitionListener; class ConfigProvider @@ -24,9 +22,6 @@ class ConfigProvider 'listeners' => [ AddConsumerDefinitionListener::class, ], - 'dependencies' => [ - IdGeneratorInterface::class => UniqidIdGenerator::class, - ], 'annotations' => [ 'scan' => [ 'paths' => [ From 63d8fd2f2d46ea7ae8d34d388ba31324307ec208 Mon Sep 17 00:00:00 2001 From: huangzhhui Date: Thu, 9 Jan 2020 18:58:55 +0800 Subject: [PATCH 9/9] Optimizd --- src/json-rpc/src/JsonRpcHttpTransporter.php | 2 +- src/json-rpc/src/JsonRpcTransporter.php | 11 ++++------- src/rpc-client/src/AbstractServiceClient.php | 2 +- src/rpc-client/src/Exception/RequestException.php | 9 +++++---- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/json-rpc/src/JsonRpcHttpTransporter.php b/src/json-rpc/src/JsonRpcHttpTransporter.php index 0a73930e7..d76489d6c 100644 --- a/src/json-rpc/src/JsonRpcHttpTransporter.php +++ b/src/json-rpc/src/JsonRpcHttpTransporter.php @@ -86,7 +86,7 @@ class JsonRpcHttpTransporter implements TransporterInterface public function recv() { - throw new \RuntimeException('JsonRpcHttpTransporter not support recv.'); + throw new \RuntimeException(__CLASS__ . ' does not support recv method.'); } public function getClient(): Client diff --git a/src/json-rpc/src/JsonRpcTransporter.php b/src/json-rpc/src/JsonRpcTransporter.php index 3b4523062..c494c9b92 100644 --- a/src/json-rpc/src/JsonRpcTransporter.php +++ b/src/json-rpc/src/JsonRpcTransporter.php @@ -88,10 +88,9 @@ class JsonRpcTransporter implements TransporterInterface return Context::get($class); } - $client = new SwooleClient(SWOOLE_SOCK_TCP); - $client->set($this->config['settings'] ?? []); - - $client = retry(2, function () use ($client) { + return Context::set($class, retry(2, function () { + $client = new SwooleClient(SWOOLE_SOCK_TCP); + $client->set($this->config['settings'] ?? []); $node = $this->getNode(); $result = $client->connect($node->host, $node->port, $this->connectTimeout); if ($result === false && ($client->errCode == 114 or $client->errCode == 115)) { @@ -100,9 +99,7 @@ class JsonRpcTransporter implements TransporterInterface throw new RuntimeException('Connect to server failed.'); } return $client; - }); - - return Context::set($class, $client); + })); } public function getLoadBalancer(): ?LoadBalancerInterface diff --git a/src/rpc-client/src/AbstractServiceClient.php b/src/rpc-client/src/AbstractServiceClient.php index 24fb41ba6..bab9f04a2 100644 --- a/src/rpc-client/src/AbstractServiceClient.php +++ b/src/rpc-client/src/AbstractServiceClient.php @@ -102,7 +102,7 @@ abstract class AbstractServiceClient protected function __request(string $method, array $params, ?string $id = null) { - if ($this->idGenerator instanceof IdGeneratorInterface && ! $id) { + if (! $id && $this->idGenerator instanceof IdGeneratorInterface) { $id = $this->idGenerator->generate(); } $response = $this->client->send($this->__generateData($method, $params, $id)); diff --git a/src/rpc-client/src/Exception/RequestException.php b/src/rpc-client/src/Exception/RequestException.php index fa483993c..f9f0a82cb 100644 --- a/src/rpc-client/src/Exception/RequestException.php +++ b/src/rpc-client/src/Exception/RequestException.php @@ -20,15 +20,16 @@ class RequestException extends \RuntimeException protected $throwable; /** - * @param $throwable = [ - * 'class' => 'RuntimeException', // The exception class + * @param $throwable + * [ + * 'class' => 'RuntimeException', // The exception class name * 'code' => 0, // The exception code * 'message' => '', // The exception message * 'attributes' => [ * 'message' => '', // The exception message * 'code' => 0, // The exception code - * 'file' => '/opt/www/hyperf/app/JsonRpc/CalculatorService.php', // The file in which the exception occurred - * 'line' => 99, // The line in which the exception occurred + * 'file' => '/opt/www/hyperf/app/JsonRpc/CalculatorService.php', // The file path which the exception occurred + * 'line' => 99, // The line of file which the exception occurred * ], * ] * @param string $message