reactphp / async

Async utilities and fibers for ReactPHP.
https://reactphp.org/async/
MIT License
204 stars 19 forks source link

Improve error reporting when incorrectly using `await()` (Value of type null is not callable in src/SimpleFiber.php:66) #50

Open greendrake opened 2 years ago

greendrake commented 2 years ago

Reproduce:

use React\EventLoop\Factory as Loop;
use React\Async;
use React\Promise\Deferred;

$loop = Loop::create();

$deferred = new Deferred;
$promise = $deferred->promise();
$loop->addTimer(0.02, function() use($deferred) {
    $deferred->resolve();
});

$foo = function() use($promise) {
    Async\await($promise);
    echo 'All good' . PHP_EOL;    
};

$loop->addTimer(0.01, $foo);

$loop->run();

How to fix it is known: wrap $foo into Async\async. But the error sucks anyway. You guys gotta find a way to get it either work anyway, or throw a nice error telling us what is wrong and hinting what to do.

SimonFrings commented 2 years ago

Hey @greendrake You're not wrong, the error message could be a bit more precise. If you already have some ideas we're always happy about pull requests. :+1:

mowses commented 1 year ago

I am having the same error: Value of type null is not callable in src/SimpleFiber.php react/async/src/SimpleFiber.php:74. Tried to do what @greendrake told: wrap into async but it does not work.

Does anyone have the solution code to share?

WyriHaximus commented 1 year ago

@mowses can you share any code that triggers this?

mowses commented 1 year ago

Update: right now I am reading your blog at https://blog.wyrihaximus.net/2021/12/async-and-await-at-the-edge-with-reactphp/

@WyriHaximus sure. I am testing my laravel-websockets functionality. Here's the code.

ConnectionTest.php

<?php

namespace Modules\Lobby\Tests\Unit\Sockets;

class ConnectionTest extends SocketTestCase
{
    public function test_debugging_error()
    {
        $this->startServer(function () {
            $this->connect("/app/INVALID-APP-KEY/room/INVALID-ROOM-ID", function () {
                dump('connection #1');
                $this->assertTrue(true);
            });
        });

        $this->startServer(function () {
            $this->connect("/app/INVALID-APP-KEY/room/INVALID-ROOM-ID", function () {
                dump('connection #2');
                $this->assertTrue(true);
            });
        });

        dd('CONNECTION #1 IS OK. BUT SCRIPT FAILS CONNECTING #2 BECAUSE OF THE `await($promise)` ERROR: Value of type null is not callable in src/SimpleFiber.php IN react/async/src/SimpleFiber.php:74');
    }
}

SocketTestCase.php

<?php

namespace Modules\Lobby\Tests\Unit\Sockets;

use BeyondCode\LaravelWebSockets\Apps\App;
use BeyondCode\LaravelWebSockets\Apps\AppProvider;
use BeyondCode\LaravelWebSockets\Facades\WebSocketsRouter;
use BeyondCode\LaravelWebSockets\Server\Logger\WebsocketsLogger;
use BeyondCode\LaravelWebSockets\Server\Router;
use BeyondCode\LaravelWebSockets\Server\WebSocketServerFactory;
use Closure;
use Mockery;
use Mockery\MockInterface;
use Modules\Lobby\Interfaces\RoomSocketHandlerInterface;
use Modules\Lobby\Sockets\RoomSocketHandler;
use Orchestra\Testbench\TestCase;
use Ratchet\Client\WebSocket;
use Ratchet\Server\IoServer;
use Symfony\Component\Console\Output\BufferedOutput;
use Tests\CreatesApplication;

abstract class SocketTestCase extends TestCase
{
    use CreatesApplication;

    protected function startServer(Closure $closure = null): IoServer
    {
        /** @var WebSocketServerFactory $server */
        $server = app(WebSocketServerFactory::class);

        /** @var Router $route */
        $route = WebSocketsRouter::getFacadeRoot();

        $route->customRoutes();  // make custom routes available by calling ->getRoutes()

        $ioServer = $server
            ->setHost(config('broadcasting.connections.pusher.options.host'))
            ->setPort(config('broadcasting.connections.pusher.options.port'))
            ->useRoutes($route->getRoutes())
            ->createServer();

        if ($closure !== null) {
            try {
                $closure($ioServer);
            } finally {
                $ioServer->socket->close();
            }
        }

        return $ioServer;
    }

    protected function connect(string $path, Closure $closure = null): void
    {
        $promise = \Ratchet\Client\connect($this->getFullConnectionString($path))
            ->then(
                function (WebSocket $conn) use ($closure) {
                    try {
                        if ($closure !== null) {
                            $closure($conn);
                        }
                    } finally {
                        $conn->close();
                    }
                }
            );

        /** @noinspection PhpUnhandledExceptionInspection */
        \React\Async\await($promise);
    }

    protected function getFullConnectionString(string $path): string
    {
        return sprintf('%s://%s:%s/%s',
            config('broadcasting.connections.pusher.options.scheme'),
            config('broadcasting.connections.pusher.options.host'),
            config('broadcasting.connections.pusher.options.port'),
            $path
        );
    }

    protected function getAppBasePath(string $appId): string
    {
        return sprintf('/app/%s',
            $this->getLobbyAppConfig($appId)->key ?? ''
        );
    }

    protected function getLobbyAppConfig(string $appId): ?App
    {
        /** @var AppProvider $provider */
        $provider = app(config('websockets.app_provider'));

        return $provider->findById($appId);
    }
}

Method connect creates a promise of a Ratchet client and then awaits for the promise to be resolved.

The error is triggered when called the next await function. As far I debugged, the second time await runs, the variable $this->$scheduler is no more null in the SimpleFiber.php which is then called the start() assigning value of null to $ret.

mowses commented 1 year ago

I managed to handle my problem. For the people who had the same issue, here's what I did to solve:

Here is the modified method:

protected function connect(string $path, Closure $closure = null): void
    {
        $loop = Loop::get();

        $promise = function () use ($path, $closure, $loop): void {
            $request = connect($this->getFullConnectionString($path), [], [], $loop)
                ->then(
                    function (WebSocket $conn) use ($closure, $loop) {
                        try {
                            if ($closure !== null) {
                                $closure($conn);
                            }
                        } finally {
                            $conn->close();
                            $loop->stop();
                        }
                    },
                    function (Throwable $e) use ($loop) {
                        $loop->stop();
                        throw $e;
                    }
                );

            await($request);
        };

        async($promise)();
        $loop->run();
    }
SimonFrings commented 1 year ago

@mowses Thanks for your input on this :+1:

Like I said above, the current error message is confusing and we want to give this a more meaningful content. @clue and I invested a few hours into a potential fix in the past, but we quickly ran into some overlaps with #65. It seems like we first need to file a new pull request in https://github.com/reactphp/event-loop in order to fix this here. We also have to define the expected behavior and write additional test cases.

Stopping the loop like you described in your example above doesn't really fix the problem here. Additionally, you shouldn't use $loop->stop() as described in https://github.com/reactphp/event-loop#stop:

This method is considered advanced usage and should be used with care. As a rule of thumb, it is usually recommended to let the loop stop only automatically when it no longer has anything to do.

We'll give an update once we filed the necessary pull requests and renamed this message to be more meaningful.

mrAndersen commented 3 weeks ago

I am getting the same error, now at assert() expression: The code:

$response = await(
                $this->browser
                    ->withResponseBuffer(1024 * 1024 * 32)
                    ->withTimeout($timeout)
                    ->request($method, $url, $headers, $body)
            );

            if (!$response) {
                $this->tracerService->incScalar('http_error');

                throw new Exception('Empty response');
            }

Thus this periodically throws Empty response because $response is NULL and when this happens I am getting:


AssertionError: assert(\is_callable($ret)) in /var/www/symfony/vendor/react/async/src/SimpleFiber.php:72
Stack trace:
#0 /var/www/symfony/vendor/react/async/src/SimpleFiber.php(72): assert(false, 'assert(\\is_call...')
#1 /var/www/symfony/vendor/react/async/src/functions.php(365): React\Async\SimpleFiber->suspend()
#2 /var/www/symfony/src/Service/Core/Http/AsyncHttpClient.php(105): React\Async\await(Object(React\Promise\Promise))

reactphp/async is 4.3.0

mrAndersen commented 3 weeks ago

Code:


$response = await(async(static function () use ($timeout, $method, $url, $headers, $body) {
                $connector = new Connector([
                    'tls' => [
                        'verify_peer' => false,
                        'verify_peer_name' => false,
                    ],
                ]);

                $browser = new Browser($connector);

                return $browser
                    ->withResponseBuffer(1024 * 1024 * 32)
                    ->withTimeout($timeout)
                    ->request($method, $url, $headers, $body)
                ;
            })());

Also produces

Error: Value of type null is not callable in /var/www/symfony/vendor/react/async/src/SimpleFiber.php:74