fix: swoft-cloud/swoft/issues/1315 not release conn on rpc error

swoft-cloud · Jun 16, 2020 · 4a7d7cb · 4a7d7cb
1 parent 82e2ec3
commit 4a7d7cb
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 43 deletions.
diff --git a/.php_cs b/.php_cs
@@ -9,25 +9,34 @@ This file is part of Swoft.
 @license  https://github.com/swoft-cloud/swoft/blob/master/LICENSE
 EOF;
 
+$rules = [
+    '@PSR2' => true,
+    'array_syntax' => [
+        'syntax' => 'short'
+    ],
+    'list_syntax' => [
+        'syntax' => 'short'
+    ],
+    'class_attributes_separation' => true,
+    'declare_strict_types' => true,
+    'global_namespace_import' => [
+        'import_constants' => true,
+        'import_functions' => true,
+    ],
+    'header_comment' => [
+        'comment_type' => 'PHPDoc',
+        'header'    => $header,
+        'separate'  => 'bottom'
+    ],
+    'no_unused_imports' => true,
+    'single_quote' => true,
+    'standardize_not_equals' => true,
+    'void_return' => true, // add :void for method
+];
+
 return PhpCsFixer\Config::create()
     ->setRiskyAllowed(true)
-    ->setRules([
-        '@PSR2' => true,
-        'array_syntax' => [
-            'syntax' => 'short'
-        ],
-        'class_attributes_separation' => true,
-        'declare_strict_types' => true,
-        'global_namespace_import' => true,
-        'header_comment' => [
-            'comment_type' => 'PHPDoc',
-            'header'    => $header,
-            'separate'  => 'bottom'
-        ],
-        'no_unused_imports' => true,
-        'single_quote' => true,
-        'standardize_not_equals' => true,
-    ])
+    ->setRules($rules)
     ->setFinder(
         PhpCsFixer\Finder::create()
             ->exclude('test')

diff --git a/src/rpc-client/src/Concern/ServiceTrait.php b/src/rpc-client/src/Concern/ServiceTrait.php
@@ -12,6 +12,7 @@
 use Swoft\Rpc\Client\ReferenceRegister;
 use Swoft\Rpc\Protocol;
 use Swoft\Stdlib\Helper\JsonHelper;
+use Throwable;
 
 /**
  * Class ServiceTrait
@@ -23,12 +24,12 @@ trait ServiceTrait
     /**
      * @param string $interfaceClass
      * @param string $methodName
-     * @param array $params
+     * @param array  $params
      *
      * @return mixed
      * @throws ConnectionPoolException
      * @throws RpcClientException
-     * @throws RpcResponseException
+     * @throws Throwable
      * @noinspection MagicMethodsValidityInspection
      */
     protected function __proxyCall(string $interfaceClass, string $methodName, array $params)
@@ -45,39 +46,46 @@ protected function __proxyCall(string $interfaceClass, string $methodName, array
         $packet = $connection->getPacket();
 
         // Ext data
-        $ext = $connection->getClient()->getExtender()->getExt();
-
-        $protocol = Protocol::new($version, $interfaceClass, $methodName, $params, $ext);
-        $data = $packet->encode($protocol);
+        $extData  = $connection->getClient()->getExtender()->getExt();
+        $protocol = Protocol::new($version, $interfaceClass, $methodName, $params, $extData);
+        $reqData  = $packet->encode($protocol);
 
+        $result  = null;
         $message = 'Rpc call failed.code=%d message=%s ' . sprintf('interface=%s method=%s pool=%s version=%s',
                 $interfaceClass, $methodName, $poolName, $version);
 
-        $result = $this->sendAndRecv($connection, $data, $message);
-        $connection->release();
+        try {
+            $rawResult = $this->sendAndRecv($connection, $reqData, $message);
+            $response  = $packet->decodeResponse($rawResult);
 
-        $response = $packet->decodeResponse($result);
-        if ($response->getError() !== null) {
-            $code = $response->getError()->getCode();
-            $message = $response->getError()->getMessage();
-            $errorData = $response->getError()->getData();
+            // Check response error
+            if ($respErr = $response->getError()) {
+                $errCode = $respErr->getCode();
+                $message = $respErr->getMessage();
+                $errData = $respErr->getData();
 
-            // Record rpc error message
-            $errorMsg = sprintf('Rpc call error!code=%d message=%s data=%s pool=%s version=%s', $code, $message,
-                JsonHelper::encode($errorData), $poolName, $version);
+                // Record rpc error message
+                $errTpl   = 'Rpc call error!code=%d message=%s pool=%s version=%s data=%s';
+                $errorMsg = sprintf($errTpl, $errCode, $message, $poolName, $version, JsonHelper::encode($errData));
 
-            Error::log($errorMsg);
+                Error::log($errorMsg);
 
-            // Only to throw message and code
-            $rpcResponseException = new RpcResponseException($message, $code);
-            // set response property
-            $rpcResponseException->setRpcResponse($response);
-            // throw exception
-            throw $rpcResponseException;
-        }
+                // Only to throw message and code
+                $rpcResponseException = new RpcResponseException($message, $errCode);
+                // set response property
+                $rpcResponseException->setRpcResponse($response);
+                // throw exception
+                throw $rpcResponseException;
+            }
 
-        return $response->getResult();
+            $result = $response->getResult();
+        } catch (Throwable $e) {
+            throw $e;
+        } finally { // NOTICE: always release resource
+            $connection->release();
+        }
 
+        return $result;
     }
 
     /**

diff --git a/src/rpc-client/src/Contract/ExtenderInterface.php b/src/rpc-client/src/Contract/ExtenderInterface.php
@@ -1,6 +1,5 @@
 <?php declare(strict_types=1);
 
-
 namespace Swoft\Rpc\Client\Contract;
 
 /**