Merge pull request #171 from clue-labs/tcp-errors

Improve TCP/IP error messages
reactphp · Sep 10, 2018 · 1ab77ec · 1ab77ec
2 parents cf7c46f + fc91b94
commit 1ab77ec
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 21 deletions.
diff --git a/src/TcpConnector.php b/src/TcpConnector.php
@@ -71,7 +71,7 @@ public function connect($uri)
         // HHVM fails to parse URIs with a query but no path, so let's simplify our URI here
         $remote = 'tcp://' . $parts['host'] . ':' . $parts['port'];
 
-        $socket = @stream_socket_client(
+        $stream = @stream_socket_client(
             $remote,
             $errno,
             $errstr,
@@ -80,39 +80,30 @@ public function connect($uri)
             stream_context_create($context)
         );
 
-        if (false === $socket) {
+        if (false === $stream) {
             return Promise\reject(new RuntimeException(
                 sprintf("Connection to %s failed: %s", $uri, $errstr),
                 $errno
             ));
         }
 
-        stream_set_blocking($socket, 0);
-
         // wait for connection
-
-        return $this->waitForStreamOnce($socket);
-    }
-
-    private function waitForStreamOnce($stream)
-    {
         $loop = $this->loop;
-
-        return new Promise\Promise(function ($resolve, $reject) use ($loop, $stream) {
-            $loop->addWriteStream($stream, function ($stream) use ($loop, $resolve, $reject) {
+        return new Promise\Promise(function ($resolve, $reject) use ($loop, $stream, $uri) {
+            $loop->addWriteStream($stream, function ($stream) use ($loop, $resolve, $reject, $uri) {
                 $loop->removeWriteStream($stream);
 
                 // The following hack looks like the only way to
                 // detect connection refused errors with PHP's stream sockets.
                 if (false === stream_socket_get_name($stream, true)) {
                     fclose($stream);
 
-                    $reject(new RuntimeException('Connection refused'));
+                    $reject(new RuntimeException('Connection to ' . $uri . ' failed: Connection refused'));
                 } else {
                     $resolve(new Connection($stream, $loop));
                 }
             });
-        }, function () use ($loop, $stream) {
+        }, function () use ($loop, $stream, $uri) {
             $loop->removeWriteStream($stream);
             fclose($stream);
 
@@ -123,7 +114,7 @@ private function waitForStreamOnce($stream)
             }
             // @codeCoverageIgnoreEnd
 
-            throw new RuntimeException('Cancelled while waiting for TCP/IP connection to be established');
+            throw new RuntimeException('Connection to ' . $uri . ' cancelled during TCP/IP handshake');
         });
     }
 }
diff --git a/tests/TcpConnectorTest.php b/tests/TcpConnectorTest.php
@@ -12,16 +12,19 @@ class TcpConnectorTest extends TestCase
 {
     const TIMEOUT = 0.1;
 
-    /** @test */
+    /**
+     * @test
+     * @expectedException RuntimeException
+     * @expectedExceptionMessage Connection to tcp://127.0.0.1:9999 failed: Connection refused
+     */
     public function connectionToEmptyPortShouldFail()
     {
         $loop = Factory::create();
 
         $connector = new TcpConnector($loop);
-        $connector->connect('127.0.0.1:9999')
-                ->then($this->expectCallableNever(), $this->expectCallableOnce());
+        $promise = $connector->connect('127.0.0.1:9999');
 
-        $loop->run();
+        Block\await($promise, $loop, self::TIMEOUT);
     }
 
     /** @test */
@@ -254,7 +257,25 @@ public function cancellingConnectionShouldRejectPromise()
         $promise = $connector->connect($server->getAddress());
         $promise->cancel();
 
-        $this->setExpectedException('RuntimeException', 'Cancelled');
+        $this->setExpectedException('RuntimeException', 'Connection to ' . $server->getAddress() . ' cancelled during TCP/IP handshake');
         Block\await($promise, $loop);
     }
+
+    public function testCancelDuringConnectionShouldNotCreateAnyGarbageReferences()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+        $connector = new TcpConnector($loop);
+        $promise = $connector->connect('127.0.0.1:9999');
+
+        $promise->cancel();
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
 }