async icon indicating copy to clipboard operation
async copied to clipboard

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

Open greendrake opened this issue 3 years ago • 8 comments

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.

greendrake avatar Jun 18 '22 08:06 greendrake

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:

SimonFrings avatar Jun 20 '22 11:06 SimonFrings

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?

mowses avatar Jul 21 '23 19:07 mowses

@mowses can you share any code that triggers this?

WyriHaximus avatar Jul 21 '23 20:07 WyriHaximus

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.

  • Once resolved, the closure from the test method is called
  • the connection is immediately closed
  • await unblocks
  • the next connection test block starts

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 avatar Jul 22 '23 10:07 mowses

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

  • Created a new Loop instance and passed as the fourth parameter of connect() function;
  • Wrapped the promise in an async function;
  • Manually run $loop->run();
  • Manually run $loop->stop(); after the request was resolved/rejected;

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();
    }

mowses avatar Jul 26 '23 14:07 mowses

@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.

SimonFrings avatar Oct 24 '23 10:10 SimonFrings