Merge pull request #128 from clue-labs/fiber-compatibility

Add fiber compatibility mode for PHP < 8.1
clue · Mar 7, 2022 · c8a4cdf · c8a4cdf
2 parents cfe0174 + 528aaaf
commit c8a4cdf
Show file tree

Hide file tree

Showing 6 changed files with 132 additions and 43 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -49,7 +49,7 @@ jobs:
       - uses: shivammathur/setup-php@v2
         with:
           php-version: ${{ matrix.php }}
-      - run: composer install --no-dev
+      - run: composer remove --dev phpunit/phpunit
       - run: php examples/index.php &
       - run: bash tests/await.sh
       - run: bash tests/acceptance.sh
@@ -71,7 +71,7 @@ jobs:
       - uses: shivammathur/setup-php@v2
         with:
           php-version: ${{ matrix.php }}
-      - run: composer install --no-dev
+      - run: composer remove --dev phpunit/phpunit
       - run: docker run -d -v "$PWD":/home/framework-x php:${{ matrix.php }}-fpm
       - run: docker run -d -p 80:80 --link $(docker ps -qn1):php -v "$PWD":/home/framework-x -v "$PWD"/examples/nginx/nginx.conf:/etc/nginx/conf.d/default.conf nginx:stable-alpine
       - run: bash tests/await.sh http://localhost
@@ -94,7 +94,7 @@ jobs:
       - uses: shivammathur/setup-php@v2
         with:
           php-version: ${{ matrix.php }}
-      - run: composer install --no-dev
+      - run: composer remove --dev phpunit/phpunit
       - run: docker run -d -p 80:80 -v "$PWD":/home/framework-x php:${{ matrix.php }}-apache sh -c "rmdir /var/www/html;ln -s /home/framework-x/examples/apache /var/www/html;ln -s /etc/apache2/mods-available/rewrite.load /etc/apache2/mods-enabled; apache2-foreground"
       - run: bash tests/await.sh http://localhost
       - run: bash tests/acceptance.sh http://localhost
@@ -116,7 +116,7 @@ jobs:
       - uses: shivammathur/setup-php@v2
         with:
           php-version: ${{ matrix.php }}
-      - run: composer install --no-dev
+      - run: composer remove --dev phpunit/phpunit
       - run: php -S localhost:8080 examples/index.php &
       - run: bash tests/await.sh
       - run: bash tests/acceptance.sh
diff --git a/docs/async/fibers.md b/docs/async/fibers.md
@@ -43,12 +43,6 @@ return value.
 
 ## Requirements
 
-> ⚠️ **Feature preview**
->
-> This is a feature preview, i.e. it might not have made it into the current beta.
-> Give feedback to help us prioritize.
-> We also welcome [contributors](../getting-started/community.md) to help out!
-
 At the moment, fibers are available as a development version by installing
 [react/async](https://github.com/reactphp/async) from a development branch
 like this:
@@ -59,24 +53,9 @@ $ composer require react/async:^4@dev
 
 Installing this package version requires PHP 8.1+ (2021-11-25) as fibers are a
 core ingredient of PHP 8.1+. We understand that adoption of this very new PHP
-version is going to take some time, so we acknowledge that this is probably one
-of the largest limitations of using fibers at the moment.
-
-But don't worry, we're committed to providing long-term support (LTS) options
-and providing a smooth upgrade path. As such, we also provide limited support
-for older PHP versions using a compatible API without taking advantage of newer
-language features. By installing the v3 development version of this package, the
-same `await()` syntax also works on PHP 7.1+ to some degree if you only have
-limited concurrency. You can install either supported development version like
-this:
-
-```bash
-$ composer require react/async:"^4@dev || ^3@dev"
-```
-
-This way, you have a much smoother upgrade path, as you can already start using
-the future API for testing and development purposes and upgrade your PHP version
-for production use at a later time.
+version is going to take some time, so we also provide a limited
+[compatibility mode](#compatibility-mode) that also works on PHP 7.1+ to ease
+upgrading.
 
 > ℹ️ **Coroutines and Promises work anywhere**
 >
@@ -145,6 +124,56 @@ Coroutines allow consuming async APIs in a way that resembles a synchronous
 code flow using the `yield` keyword. You can also directly use promises as a
 core building block used in all our async APIs for maximum performance.
 
+### Compatibility mode
+
+Fibers are a core ingredient of PHP 8.1+, but the same syntax also works on
+older PHP versions to some degree if you only have limited concurrency.
+
+For production usage, we highly recommend using PHP 8.1+. At the moment, fibers
+are available as a development version by installing
+[react/async](https://github.com/reactphp/async) from a development branch
+like this:
+
+```bash
+$ composer require react/async:^4@dev
+```
+
+Installing this package version requires PHP 8.1+ (2021-11-25) as fibers are a
+core ingredient of PHP 8.1+. We understand that adoption of this very new PHP
+version is going to take some time, so we acknowledge that this is probably one
+of the largest limitations of using fibers at the moment.
+
+But don't worry, we're committed to providing long-term support (LTS) options
+and providing a smooth upgrade path. As such, we also provide limited support
+for older PHP versions using a compatible API without taking advantage of newer
+language features. By installing the v3 development version of this package, the
+same `await()` syntax also works on PHP 7.1+ to some degree if you only have
+limited concurrency. You can install either supported development version like
+this:
+
+```bash
+$ composer require react/async:"^4@dev || ^3@dev"
+```
+
+This way, you have a much smoother upgrade path, as you can already start using
+the future API for testing and development purposes and upgrade your PHP version
+for production use at a later time.
+
+> ⚠️ **Production usage**
+>
+> For production usage, we highly recommend using PHP 8.1+. If you're using the
+> `await()` function in compatibility mode, it may stop the loop from running and
+> may print a warning like this:
+>
+> ```
+> Warning: Loop restarted. Upgrade to react/async v4 recommended […]
+> ```
+>
+> Internally, the compatibility mode will cause recursive loop executions when
+> dealing with concurrent requests. This should work fine for development
+> purposes and fast controllers with low concurrency, but may cause issues in
+> production with high concurrency.
+
 ### How do fibers work?
 
 Fibers are a means of creating code blocks that can be paused and resumed, but

diff --git a/examples/index.php b/examples/index.php
@@ -24,21 +24,19 @@
     );
 });
 
-$app->get('/sleep/promise', function () {
-    return React\Promise\Timer\sleep(0.1)->then(function () {
-        return React\Http\Message\Response::plaintext("OK\n");
-    });
+$app->get('/sleep/fiber', function () {
+    React\Async\await(React\Promise\Timer\sleep(0.1));
+    return React\Http\Message\Response::plaintext("OK\n");
 });
 $app->get('/sleep/coroutine', function () {
     yield React\Promise\Timer\sleep(0.1);
     return React\Http\Message\Response::plaintext("OK\n");
 });
-if (PHP_VERSION_ID >= 80100 && function_exists('React\Async\async')) { // requires PHP 8.1+ with react/async 4+
-    $app->get('/sleep/fiber', function () {
-        React\Async\await(React\Promise\Timer\sleep(0.1));
+$app->get('/sleep/promise', function () {
+    return React\Promise\Timer\sleep(0.1)->then(function () {
         return React\Http\Message\Response::plaintext("OK\n");
     });
-}
+});
 
 $app->get('/uri[/{path:.*}]', function (ServerRequestInterface $request) {
     return React\Http\Message\Response::plaintext(

diff --git a/src/App.php b/src/App.php
@@ -180,8 +180,6 @@ public function run()
         } else {
             $this->runOnce(); // @codeCoverageIgnore
         }
-
-        Loop::run();
     }
 
     private function runLoop()
@@ -208,6 +206,13 @@ private function runLoop()
 
             \fwrite(STDERR, (string)$orig);
         });
+
+        do {
+            Loop::run();
+
+            // Fiber compatibility mode for PHP < 8.1: Restart loop as long as socket is available
+            $this->sapi->log('Warning: Loop restarted. Upgrade to react/async v4 recommended for production use.');
+        } while ($socket->getAddress() !== null);
     }
 
     private function runOnce()
@@ -223,6 +228,8 @@ private function runOnce()
                 $this->sapi->sendResponse($response);
             });
         }
+
+        Loop::run();
     }
 
     /**

diff --git a/tests/AppTest.php b/tests/AppTest.php
@@ -171,7 +171,7 @@ public function testRunWillReportListeningAddressAndRunLoopWithSocketServer()
             fclose($socket);
         });
 
-        $this->expectOutputRegex('/' . preg_quote('Listening on http://127.0.0.1:8080' . PHP_EOL, '/') . '$/');
+        $this->expectOutputRegex('/' . preg_quote('Listening on http://127.0.0.1:8080' . PHP_EOL, '/') . '.*/');
         $app->run();
     }
 
@@ -193,7 +193,7 @@ public function testRunWillReportListeningAddressFromEnvironmentAndRunLoopWithSo
             fclose($socket);
         });
 
-        $this->expectOutputRegex('/' . preg_quote('Listening on http://' . $addr . PHP_EOL, '/') . '$/');
+        $this->expectOutputRegex('/' . preg_quote('Listening on http://' . $addr . PHP_EOL, '/') . '.*/');
         $app->run();
     }
 
@@ -211,7 +211,7 @@ public function testRunWillReportListeningAddressFromEnvironmentWithRandomPortAn
             fclose($socket);
         });
 
-        $this->expectOutputRegex('/' . preg_quote('Listening on http://127.0.0.1:', '/') . '\d+' . PHP_EOL . '$/');
+        $this->expectOutputRegex('/' . preg_quote('Listening on http://127.0.0.1:', '/') . '\d+' . PHP_EOL . '.*/');
         $app->run();
     }
 
@@ -838,6 +838,34 @@ public function testHandleRequestWithMatchingRouteReturnsPendingPromiseWhenHandl
         $this->assertFalse($resolved);
     }
 
+    public function testHandleRequestWithMatchingRouteReturnsResponseWhenHandlerReturnsResponseAfterAwaitingPromiseResolvingWithResponse()
+    {
+        $app = $this->createAppWithoutLogger();
+
+        $app->get('/users', function () {
+            return await(resolve(new Response(
+                200,
+                [
+                    'Content-Type' => 'text/html'
+                ],
+                "OK\n"
+            )));
+        });
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        // $response = $app->handleRequest($request);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $response = $ref->invoke($app, $request);
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(200, $response->getStatusCode());
+        $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
+        $this->assertEquals("OK\n", (string) $response->getBody());
+    }
+
     public function testHandleRequestWithMatchingRouteReturnsPromiseResolvingWithResponseWhenHandlerReturnsResponseAfterAwaitingPromiseResolvingWithResponse()
     {
         if (PHP_VERSION_ID < 80100 || !function_exists('React\Async\async')) {
@@ -1111,6 +1139,33 @@ public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWit
         $this->assertStringContainsString("<p>Expected request handler to return <code>Psr\Http\Message\ResponseInterface</code> but got uncaught <code>RuntimeException</code> with message <code>Foo</code> in <code title=\"See " . __FILE__ . " line $line\">AppTest.php:$line</code>.</p>\n", (string) $response->getBody());
     }
 
+    public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerThrowsAfterAwaitingPromiseRejectingWithException()
+    {
+        $app = $this->createAppWithoutLogger();
+
+        $line = __LINE__ + 2;
+        $app->get('/users', function () {
+            return await(reject(new \RuntimeException('Foo')));
+        });
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        // $response = $app->handleRequest($request);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $response = $ref->invoke($app, $request);
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(500, $response->getStatusCode());
+        $this->assertEquals('text/html; charset=utf-8', $response->getHeaderLine('Content-Type'));
+        $this->assertStringMatchesFormat("<!DOCTYPE html>\n<html>%a</html>\n", (string) $response->getBody());
+
+        $this->assertStringContainsString("<title>Error 500: Internal Server Error</title>\n", (string) $response->getBody());
+        $this->assertStringContainsString("<p>The requested page failed to load, please try again later.</p>\n", (string) $response->getBody());
+        $this->assertStringContainsString("<p>Expected request handler to return <code>Psr\Http\Message\ResponseInterface</code> but got uncaught <code>RuntimeException</code> with message <code>Foo</code> in <code title=\"See " . __FILE__ . " line $line\">AppTest.php:$line</code>.</p>\n", (string) $response->getBody());
+    }
+
     public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWithInternalServerErrorResponseWhenHandlerThrowsAfterAwaitingPromiseRejectingWithException()
     {
         if (PHP_VERSION_ID < 80100 || !function_exists('React\Async\async')) {

diff --git a/tests/acceptance.sh b/tests/acceptance.sh
@@ -26,9 +26,9 @@ out=$(curl -v $base/ 2>&1 -X POST); match "HTTP/.* 405"
 out=$(curl -v $base/error 2>&1);        match "HTTP/.* 500" && match -iP "Content-Type: text/html; charset=utf-8[\r\n]" && match "<code>Unable to load error</code>"
 out=$(curl -v $base/error/null 2>&1);   match "HTTP/.* 500" && match -iP "Content-Type: text/html; charset=utf-8[\r\n]"
 
-out=$(curl -v $base/sleep/promise 2>&1);    match "HTTP/.* 200" && match -iP "Content-Type: text/plain; charset=utf-8[\r\n]"
+out=$(curl -v $base/sleep/fiber 2>&1);      match "HTTP/.* 200" && match -iP "Content-Type: text/plain; charset=utf-8[\r\n]"
 out=$(curl -v $base/sleep/coroutine 2>&1);  match "HTTP/.* 200" && match -iP "Content-Type: text/plain; charset=utf-8[\r\n]"
-out=$(curl -v $base/sleep/fiber 2>&1);      skipif "HTTP/.* 404" && match "HTTP/.* 200" && match -iP "Content-Type: text/plain; charset=utf-8[\r\n]" # skip PHP < 8.1
+out=$(curl -v $base/sleep/promise 2>&1);    match "HTTP/.* 200" && match -iP "Content-Type: text/plain; charset=utf-8[\r\n]"
 
 out=$(curl -v $base/uri 2>&1);                          match "HTTP/.* 200" && match "$base/uri"
 out=$(curl -v $base/uri/ 2>&1);                         match "HTTP/.* 200" && match "$base/uri/"