Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
greendrake opened this issue Jun 18, 2022 · 6 comments
Labels
help wanted Extra attention is needed new feature New feature or request

Comments

@greendrake
Copy link

greendrake commented Jun 18, 2022

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
Copy link
Member

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

@clue clue added this to the v4.0.0 milestone Jun 20, 2022
@clue clue changed the title Value of type null is not callable in src/SimpleFiber.php:66 Improve error reporting when incorrectly using await() (Value of type null is not callable in src/SimpleFiber.php:66) Jun 20, 2022
@clue clue added new feature New feature or request help wanted Extra attention is needed labels Jun 20, 2022
@clue clue removed this from the v4.0.0 milestone Jul 11, 2022
@mowses
Copy link

mowses commented Jul 21, 2023

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
Copy link
Member

@mowses can you share any code that triggers this?

@mowses
Copy link

mowses commented Jul 22, 2023

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
Copy link

mowses commented Jul 26, 2023

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

@SimonFrings
Copy link
Member

SimonFrings commented Oct 24, 2023

@mowses Thanks for your input on this 👍

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed new feature New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants