Merge pull request #41 from phpinnacle/reconnect_heartbeat_fixes

* Bug fix of heartbeat watcher. Throw received exception on client close event. Update dependencies * php 8.1 or higher is required --------- Co-authored-by: Stepan Zolotarev <[email protected]>
phpinnacle · Apr 22, 2024 · 3111d38 · 3111d38
2 parents 96ff3d8 + 7c68063
commit 3111d38
Show file tree

Hide file tree

Showing 14 changed files with 68 additions and 58 deletions.
diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
@@ -13,7 +13,7 @@ jobs:
       - name: Install PHP
         uses: shivammathur/setup-php@v2
         with:
-          php-version: 8.0
+          php-version: 8.1
           coverage: none
           tools: composer:v2
 
@@ -33,7 +33,7 @@ jobs:
       - name: Install PHP
         uses: shivammathur/setup-php@v2
         with:
-          php-version: 8.0
+          php-version: 8.1
           coverage: none
           tools: composer:v2
 
@@ -68,7 +68,7 @@ jobs:
       - name: Install PHP with extensions
         uses: shivammathur/setup-php@v2
         with:
-          php-version: 8.0
+          php-version: 8.1
           extensions: ${{ env.PHP_EXTENSIONS }}
           ini-values: ${{ env.PHP_INI_VALUES }}
           tools: composer:v2

diff --git a/composer.json b/composer.json
@@ -18,16 +18,16 @@
     }
   ],
   "require": {
-    "php": ">=8.0",
+    "php": ">=8.1",
     "amphp/amp": "v2.6.*",
     "amphp/socket": "v1.2.*",
     "phpinnacle/buffer": "v1.2.*",
     "evenement/evenement": "v3.0.*"
   },
   "require-dev": {
-    "phpunit/phpunit": "v9.5.*",
-    "vimeo/psalm": "v4.19.*",
-    "phpstan/phpstan": "v1.4.*"
+    "phpunit/phpunit": "10.* || 11.*",
+    "vimeo/psalm": "5.*",
+    "phpstan/phpstan": "^1.10"
   },
   "prefer-stable": true,
   "autoload": {
@@ -43,7 +43,7 @@
   "scripts": {
     "psalm": "./vendor/bin/psalm --config=psalm.xml",
     "phpstan": "./vendor/bin/phpstan analyse src --level 9",
-    "tests": "./vendor/bin/phpunit --configuration phpunit.xml --verbose",
+    "tests": "./vendor/bin/phpunit --configuration phpunit.xml",
     "coverage": "./vendor/bin/phpunit --configuration phpunit.xml --coverage-html ./coverage --verbose"
   },
   "extra": {

diff --git a/phpunit.xml b/phpunit.xml
@@ -1,25 +1,29 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" bootstrap="vendor/autoload.php" backupGlobals="false"
-         backupStaticAttributes="false" beStrictAboutTestsThatDoNotTestAnything="false" colors="true" verbose="true"
-         convertErrorsToExceptions="true" convertNoticesToExceptions="true" convertWarningsToExceptions="true"
-         failOnRisky="true" failOnWarning="true" stopOnFailure="false"
-         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
-    <coverage>
-        <include>
-            <directory>./</directory>
-        </include>
-        <exclude>
-            <directory>./tests</directory>
-            <directory>./vendor</directory>
-        </exclude>
-    </coverage>
-    <php>
-        <ini name="error_reporting" value="-1"/>
-        <env name="RIDGE_TEST_DSN" value="amqp://guest:[email protected]:5672/?heartbeat=0"/>
-    </php>
-    <testsuites>
-        <testsuite name="PHPinnacle Test Suite">
-            <directory>./tests/</directory>
-        </testsuite>
-    </testsuites>
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         bootstrap="vendor/autoload.php"
+         backupGlobals="false"
+         beStrictAboutTestsThatDoNotTestAnything="false"
+         colors="true"
+         failOnRisky="true"
+         failOnWarning="true"
+         stopOnFailure="false"
+         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.5/phpunit.xsd">
+  <php>
+    <ini name="error_reporting" value="-1"/>
+    <env name="RIDGE_TEST_DSN" value="amqp://guest:[email protected]:5672/?heartbeat=0"/>
+  </php>
+  <testsuites>
+    <testsuite name="PHPinnacle Test Suite">
+      <directory>./tests/</directory>
+    </testsuite>
+  </testsuites>
+  <source>
+    <include>
+      <directory>./</directory>
+    </include>
+    <exclude>
+      <directory>./tests</directory>
+      <directory>./vendor</directory>
+    </exclude>
+  </source>
 </phpunit>
diff --git a/psalm.xml b/psalm.xml
@@ -1,7 +1,5 @@
 <?xml version="1.0"?>
 <psalm
-        totallyTyped="true"
-        useDocblockTypes="true"
         useDocblockPropertyTypes="true"
         strictBinaryOperands="true"
         findUnusedPsalmSuppress="true"
@@ -21,5 +19,6 @@
     <issueHandlers>
         <PropertyNotSetInConstructor errorLevel="suppress"/>
         <UnnecessaryVarAnnotation errorLevel="suppress"/>
+        <RiskyTruthyFalsyComparison errorLevel="suppress"/>
     </issueHandlers>
 </psalm>
diff --git a/src/Channel.php b/src/Channel.php
@@ -149,6 +149,9 @@ function () use ($outOfBand) {
     }
 
     /**
+     * @psalm-suppress InvalidReturnType
+     * @psalm-suppress InvalidReturnStatement
+     *
      * @return Promise<void>
      *
      * @throws \PHPinnacle\Ridge\Exception\ChannelException
@@ -1148,9 +1151,12 @@ public function doPublish
         $buffer->appendUint8(206);
 
         if (!empty($body)) {
-            /* @phpstan-ignore-next-line */
+            /**
+             * @phpstan-ignore-next-line
+             * @psalm-suppress ArgumentTypeCoercion
+             */
             $chunks = \str_split($body, $this->properties->maxFrame());
-
+            /** @psalm-suppress RedundantConditionGivenDocblockType */
             if ($chunks !== false) {
                 foreach ($chunks as $chunk) {
                     $buffer
@@ -1197,6 +1203,8 @@ static function (Protocol\AbstractFrame $frame) use ($deferred) {
     public function forceClose(\Throwable $exception): void
     {
         if ($this->state !== self::STATE_CLOSED) {
+            $this->connection->cancel($this->id);
+
             $this->state = self::STATE_CLOSED;
             $this->commandWaitQueue->cancel($exception);
             $this->emit(self::EVENT_CHANNEL_CLOSED, [$exception]);

diff --git a/src/Client.php b/src/Client.php
@@ -83,6 +83,8 @@ public function __construct(Config $config)
                 }
                 $this->channels = [];
                 $this->commandWaitQueue->cancel($exception);
+
+                throw $exception;
             }
         });
     }
@@ -186,6 +188,9 @@ function(): void
     }
 
     /**
+     * @psalm-suppress InvalidReturnType
+     * @psalm-suppress InvalidReturnStatement
+     *
      * @return Promise<void>
      *
      * @throws \PHPinnacle\Ridge\Exception\ClientException

diff --git a/src/Connection.php b/src/Connection.php
@@ -160,9 +160,12 @@ function () {
                                 $this->lastRead = Loop::now();
 
                                 /**
-                                 * @psalm-var callable(AbstractFrame):Promise<bool> $callback
+                                 * @psalm-suppress PossiblyInvalidArgument
+                                 *
+                                 * @var callable(AbstractFrame):Promise<bool> $callback
                                  */
                                 foreach ($this->callbacks[(int)$frame->channel][$class] ?? [] as $i => $callback) {
+                                    /** @phpstan-ignore-next-line */
                                     if (yield call($callback, $frame)) {
                                         unset($this->callbacks[(int)$frame->channel][$class][$i]);
                                     }
@@ -187,9 +190,9 @@ public function heartbeat(int $timeout): void
          * We run the callback even more often to avoid race conditions if the loop is a bit under pressure
          * otherwise we could miss heartbeats in rare conditions
          */
-        $interval = $timeout / 2;
+        $interval = (int) ($timeout / 2);
         $this->heartbeatWatcherId = Loop::repeat(
-            $interval / 3,
+            (int) ($interval / 3),
             function (string $watcherId) use ($interval, $timeout){
                 $currentTime = Loop::now();
 

diff --git a/tests/AsyncTest.php → tests/AsyncTestCase.php b/tests/AsyncTest.php → tests/AsyncTestCase.php
@@ -13,27 +13,17 @@
 use Amp\Loop;
 use function Amp\call;
 
-abstract class AsyncTest extends RidgeTest
+abstract class AsyncTestCase extends RidgeTestCase
 {
     /**
-     * @var string
+     * @var string|null
      */
     private $realTestName;
 
-    /**
-     * @codeCoverageIgnore Invoked before code coverage data is being collected.
-     *
-     * @param string $name
-     */
-    public function setName(string $name): void
+    protected function runTest(): mixed
     {
-        parent::setName($name);
+        $this->realTestName = $this->name();
 
-        $this->realTestName = $name;
-    }
-
-    protected function runTest()
-    {
         parent::setName('runTestAsync');
 
         return parent::runTest();
@@ -49,6 +39,7 @@ protected function runTestAsync(...$args)
 
                 yield $client->connect();
 
+                $args = $args ?: [];
                 \array_unshift($args, $client);
 
                 $return = yield call([$this, $this->realTestName], ...$args);

diff --git a/tests/BufferTest.php b/tests/BufferTest.php
@@ -13,7 +13,7 @@
 use PHPinnacle\Ridge\Buffer;
 use PHPinnacle\Ridge\Exception;
 
-class BufferTest extends RidgeTest
+class BufferTest extends RidgeTestCase
 {
     public function testTimestamp()
     {

diff --git a/tests/ChannelTest.php b/tests/ChannelTest.php
@@ -18,7 +18,7 @@
 use PHPinnacle\Ridge\Message;
 use PHPinnacle\Ridge\Queue;
 
-class ChannelTest extends AsyncTest
+class ChannelTest extends AsyncTestCase
 {
     public function testOpenNotReadyChannel(Client $client)
     {

diff --git a/tests/ClientConnectTest.php b/tests/ClientConnectTest.php
@@ -16,7 +16,7 @@
 use PHPinnacle\Ridge\Client;
 use PHPinnacle\Ridge\Message;
 
-class ClientConnectTest extends RidgeTest
+class ClientConnectTest extends RidgeTestCase
 {
     public function testConnect()
     {

diff --git a/tests/ClientTest.php b/tests/ClientTest.php
@@ -14,7 +14,7 @@
 use PHPinnacle\Ridge\Client;
 use PHPinnacle\Ridge\Message;
 
-class ClientTest extends AsyncTest
+class ClientTest extends AsyncTestCase
 {
     public function testOpenChannel(Client $client)
     {

diff --git a/tests/ConfigTest.php → tests/ConfigTestCase.php b/tests/ConfigTest.php → tests/ConfigTestCase.php
@@ -12,7 +12,7 @@
 
 use PHPinnacle\Ridge\Config;
 
-class ConfigTest extends RidgeTest
+class ConfigTestCase extends RidgeTestCase
 {
     public function testCreate()
     {

diff --git a/tests/RidgeTest.php → tests/RidgeTestCase.php b/tests/RidgeTest.php → tests/RidgeTestCase.php
@@ -15,7 +15,7 @@
 use PHPinnacle\Ridge\Config;
 use PHPUnit\Framework\TestCase;
 
-abstract class RidgeTest extends TestCase
+abstract class RidgeTestCase extends TestCase
 {
     /**
      * @param mixed $value