Skip to content
This repository was archived by the owner on Feb 7, 2024. It is now read-only.

Multiple Websockets routes #412

Closed
Tofandel opened this issue Jul 4, 2020 · 3 comments
Closed

Multiple Websockets routes #412

Tofandel opened this issue Jul 4, 2020 · 3 comments
Labels
postponed Will be postponed until further notice

Comments

@Tofandel
Copy link
Contributor

Tofandel commented Jul 4, 2020

I have multiple websockets handlers and when defining the routes, I can indeed only access the defined routes the rest will return 404 but for some reason only the last defined controller is called.. (Can check by dumping on the onOpen handler)

WebSocketsRouter::webSocket('ws/site', \App\Http\WebSocket\Site::class); //The admin controller is used when accessing /ws/site
WebSocketsRouter::webSocket('ws/admin', \App\Http\WebSocket\Admin::class);

Am I missing something or is there a huge bug there?

@Tofandel
Copy link
Contributor Author

Tofandel commented Jul 4, 2020

I have debugged this and there is indeed a huge bug due to the logger being a singleton and the decoration overriding the app on the singleton instance

In src/Server/Router.php

protected function createWebSocketsServer(string $action): WsServer
    {
        $app = app($action);

        if (WebsocketsLogger::isEnabled()) {
            $app = WebsocketsLogger::decorate($app); //This is a singleton and that means calling the method multiple time will override the previous app
        }

        return new WsServer($app);
    }

The websocket logger needs to be refactored, it cannot be a singleton or it needs to encapsulate the router instead of the App.

There is no way to disable the logger other than to disable the entire debug of laravel.. So no viable workaround other than to disable debug.. Which I don't want to do while developing

Tofandel added a commit to Tofandel/laravel-websockets that referenced this issue Jul 4, 2020
Tofandel added a commit to Tofandel/laravel-websockets that referenced this issue Jul 4, 2020
Tofandel added a commit to Tofandel/laravel-websockets that referenced this issue Jul 4, 2020
@Tofandel
Copy link
Contributor Author

Tofandel commented Jul 4, 2020

#414 Correctly fixes the issue by cloning the singleton on decoration, probably not the best approach but the only fix I could come up with without refactoring the entire WebSocket logger class

@rennokki rennokki added the postponed Will be postponed until further notice label Aug 13, 2020
rennokki added a commit that referenced this issue Aug 19, 2020
Fix multiple routes not working when Logger enabled #412
@rennokki
Copy link
Collaborator

@Tofandel Indeed this was an actual problem. I have tagged 1.6.1 with this fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
postponed Will be postponed until further notice
Projects
None yet
Development

No branches or pull requests

2 participants