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

[WIP] Enable WebSockets via Swoole #977

Closed
wants to merge 5 commits into from

Conversation

kingIZZZY
Copy link

@kingIZZZY kingIZZZY commented Dec 23, 2024

[WIP] Laravel Octane Swoole WebSockets

Already using Laravel Octane via Swoole?

Take full advantage of Swoole built-in WebSockets features to serve Laravel powered WebSockets!

Configure octane.swoole.enableWebSockets to true to enable

@kingIZZZY
Copy link
Author

To access the Swoole server object from laravel code - here is one way how:

app('Laravel\Octane\Swoole\WorkerState')->server

For example to push WebSocket messages to clients:

app('Laravel\Octane\Swoole\WorkerState')->server->push($clientSocketFD, $message);

Is this the 1) only way? 2) correct way? If anyone knows - please share 🙏

*/
public function handleWebSocketMessage(\Swoole\WebSocket\Server $server, \Swoole\WebSocket\Frame $frame)
{
\Laravel\Octane\CurrentApplication::set($sandbox = clone $this->app);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty much copied from handleTick/handleTask, does it make sense? Should it be doing these same steps for incoming WebSocket messages/disconnect?

@taylorotwell
Copy link
Member

Would need to be Echo compatible IMO.

@kingIZZZY
Copy link
Author

kingIZZZY commented Dec 23, 2024

Could we keep the PR open for a bit longer to 1) maybe hear some more opinions 2) maybe attract some more attention of others better well versed with Echo than myself who can take up contributing on that aspect?

Or is there an evident impossibility of compatibility with Echo which dooms this whole approach/effort that I am missing?

@goodevilgenius
Copy link

Personally, I think this looks like a really good start to me.

Only suggestion I have is that I don't think the union type for Server|WServer is necessary. Last time I checked, Swoole\WebSocket\Server extends Swoole\Http\Server. So, you can just type check the Http server and that covers both.

Making it work with echo would be the logical next step, though.

@kingIZZZY
Copy link
Author

Oh no 😭 I think I might abandon this effort for now
It seems this feature is prone to throwing this exception possibly among others:

  • BindingResolutionException: Target class [events] does not exist.

Because: Laravel does not support Swoole Coroutines

Seems it might be a deal breaker for now, oh well - it's back to Hyperf for me...
Anyone who'd like can also check out the Laravel-Hyperf project (see the above linked issue #765 and specific comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants