Merge pull request #178 from clue-labs/unhandled-rejections

Update close handler to avoid unhandled promise rejections

Author: WyriHaximus

composer.json

@@ -8,7 +8,7 @@
         "evenement/evenement": "^3.0 || ^2.1 || ^1.1",
         "react/event-loop": "^1.2",
         "react/promise": "^3 || ^2.7",
-        "react/promise-stream": "^1.4",
+        "react/promise-stream": "^1.6",
         "react/promise-timer": "^1.9",
         "react/socket": "^1.12"
     },

src/Factory.php

@@ -202,6 +202,8 @@ public function createConnection(
             // either close successful connection or cancel pending connection attempt
             $connecting->then(function (SocketConnectionInterface $connection) {
                 $connection->close();
+            }, function () {
+                // ignore to avoid reporting unhandled rejection
             });
             $connecting->cancel();
         });

src/Io/LazyConnection.php

@@ -220,6 +220,8 @@ public function close()
         if ($this->connecting !== null) {
             $this->connecting->then(function (ConnectionInterface $connection) {
                 $connection->close();
+            }, function () {
+                // ignore to avoid reporting unhandled rejection
             });
             if ($this->connecting !== null) {
                 $this->connecting->cancel();

tests/FactoryTest.php

@@ -279,7 +279,7 @@ public function testConnectWithValidAuthWillRunUntilQuit()
             $connection->quit()->then(function () {
                 echo 'closed.';
             });
-        }, 'printf')->then(null, 'printf');
+        });
 
         Loop::run();
     }
@@ -296,7 +296,7 @@ public function testConnectWithValidAuthAndWithoutDbNameWillRunUntilQuit()
             $connection->quit()->then(function () {
                 echo 'closed.';
             });
-        }, 'printf')->then(null, 'printf');
+        });
 
         Loop::run();
     }
@@ -313,7 +313,7 @@ public function testConnectWithValidAuthWillIgnoreNegativeTimeoutAndRunUntilQuit
             $connection->quit()->then(function () {
                 echo 'closed.';
             });
-        }, 'printf')->then(null, 'printf');
+        });
 
         Loop::run();
     }
@@ -333,8 +333,7 @@ public function testConnectWithValidAuthCanPingAndThenQuit()
                     echo 'closed.';
                 });
             });
-
-        }, 'printf')->then(null, 'printf');
+        });
 
         Loop::run();
     }
@@ -354,14 +353,14 @@ public function testConnectWithValidAuthCanQueuePingAndQuit()
             $connection->quit()->then(function () {
                 echo 'closed.';
             });
-        }, 'printf')->then(null, 'printf');
+        });
 
         Loop::run();
     }
 
     public function testConnectWithValidAuthQuitOnlyOnce()
     {
-        $this->expectOutputString('connected.closed.');
+        $this->expectOutputString('connected.rejected.closed.');
 
         $factory = new Factory();
 
@@ -372,9 +371,11 @@ public function testConnectWithValidAuthQuitOnlyOnce()
                 echo 'closed.';
             });
             $connection->quit()->then(function () {
-                echo 'closed.';
+                echo 'never reached.';
+            }, function () {
+                echo 'rejected.';
             });
-        }, 'printf')->then(null, 'printf');
+        });
 
         Loop::run();
     }
@@ -397,7 +398,7 @@ public function testConnectWithValidAuthCanCloseOnlyOnce()
 
             $connection->close();
             $connection->close();
-        }, 'printf')->then(null, 'printf');
+        });
 
         Loop::run();
     }
@@ -425,7 +426,7 @@ public function testConnectWithValidAuthCanCloseAndAbortPing()
                 echo 'aborted queued (' . $e->getMessage() . ').';
             });
             $connection->close();
-        }, 'printf')->then(null, 'printf');
+        });
 
         Loop::run();
     }
@@ -626,7 +627,7 @@ public function testConnectLazyWithInvalidAuthWillRejectPingButWillNotEmitErrorO
 
     public function testConnectLazyWithValidAuthWillPingBeforeQuitButNotAfter()
     {
-        $this->expectOutputString('ping.closed.');
+        $this->expectOutputString('rejected.ping.closed.');
 
         $factory = new Factory();
 
@@ -643,6 +644,8 @@ public function testConnectLazyWithValidAuthWillPingBeforeQuitButNotAfter()
 
         $connection->ping()->then(function () {
             echo 'never reached';
+        }, function () {
+            echo 'rejected.';
         });
 
         Loop::run();

tests/Io/LazyConnectionTest.php

@@ -24,10 +24,13 @@ public function testPingWillNotCloseConnectionWhenPendingConnectionFails()
         $connection->on('error', $this->expectCallableNever());
         $connection->on('close', $this->expectCallableNever());
 
-        $connection->ping();
+        $promise = $connection->ping();
+
+        $promise->then(null, $this->expectCallableOnce()); // avoid reporting unhandled rejection
 
         $deferred->reject(new \RuntimeException());
     }
+
     public function testPingWillNotCloseConnectionWhenUnderlyingConnectionCloses()
     {
         $base = $this->getMockBuilder('React\MySQL\Io\LazyConnection')->setMethods(['ping'])->disableOriginalConstructor()->getMock();
@@ -678,7 +681,10 @@ public function testCloseAfterPingDoesNotEmitConnectionErrorFromAbortedConnectio
         $connection->on('error', $this->expectCallableNever());
         $connection->on('close', $this->expectCallableOnce());
 
-        $connection->ping();
+        $promise = $connection->ping();
+
+        $promise->then(null, $this->expectCallableOnce()); // avoid reporting unhandled rejection
+
         $connection->close();
     }
 

tests/ResultQueryTest.php

@@ -17,8 +17,6 @@ public function testSelectStaticText()
             $this->assertCount(1, $command->resultRows);
             $this->assertCount(1, $command->resultRows[0]);
             $this->assertSame('foo', reset($command->resultRows[0]));
-
-            $this->assertInstanceOf('React\MySQL\Connection', $conn);
         });
 
         $connection->quit();
@@ -57,7 +55,7 @@ public function testSelectStaticValueWillBeReturnedAsIs($value)
             $this->assertCount(1, $command->resultRows);
             $this->assertCount(1, $command->resultRows[0]);
             $this->assertSame($expected, reset($command->resultRows[0]));
-        })->then(null, 'printf');
+        });
 
         $connection->quit();
         Loop::run();
@@ -82,7 +80,7 @@ public function testSelectStaticValueWillBeReturnedAsIsWithNoBackslashEscapesSql
             $this->assertCount(1, $command->resultRows);
             $this->assertCount(1, $command->resultRows[0]);
             $this->assertSame($expected, reset($command->resultRows[0]));
-        })->then(null, 'printf');
+        });
 
         $connection->quit();
         Loop::run();
@@ -138,7 +136,7 @@ public function testSelectLongStaticTextHasTypeStringWithValidLength()
 
         $connection->query('SELECT ?', [$value])->then(function (QueryResult $command) use ($length) {
             $this->assertCount(1, $command->resultFields);
-            $this->assertEquals($length * 3, $command->resultFields[0]['length']);
+            $this->assertEquals($length * 4, $command->resultFields[0]['length']);
             $this->assertSame(Constants::FIELD_TYPE_VAR_STRING, $command->resultFields[0]['type']);
         });
 
@@ -430,7 +428,7 @@ public function testInvalidSelectShouldFail()
 
         $connection->query('select * from invalid_table')->then(
             $this->expectCallableNever(),
-            function (\Exception $error) {
+            function (\Exception $error) use ($db) {
                 $this->assertEquals("Table '$db.invalid_table' doesn't exist", $error->getMessage());
             }
         );
@@ -446,7 +444,13 @@ public function testInvalidMultiStatementsShouldFailToPreventSqlInjections()
         $connection->query('select 1;select 2;')->then(
             $this->expectCallableNever(),
             function (\Exception $error) {
-                $this->assertContains("You have an error in your SQL syntax", $error->getMessage());
+                if (method_exists($this, 'assertStringContainsString')) {
+                    // PHPUnit 9+
+                    $this->assertStringContainsString("You have an error in your SQL syntax", $error->getMessage());
+                } else {
+                    // legacy PHPUnit < 9
+                    $this->assertContains("You have an error in your SQL syntax", $error->getMessage());
+                }
             }
         );