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

[BUG] - Websockets implementation is broken when using API_KEY #563

Closed
zioproto opened this issue Nov 13, 2023 · 5 comments
Closed

[BUG] - Websockets implementation is broken when using API_KEY #563

zioproto opened this issue Nov 13, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@zioproto
Copy link
Contributor

Describe the bug
The current Websockets implementation is broken when using API_KEY, it has issues accepting a broken trying to reconnect.

The core container prints this error message:

cheshire_cat_core           | ERROR:    ASGI callable returned without sending handshake.

To Reproduce
Steps to reproduce the behavior:

  1. Start the core container with the API_KEY
  2. Use the /admin path and start using the cat (enter the API_KEY when prompted by the web interface)
  3. Refresh the page, enter the API_KEY again in the pop-up window.
  4. Observe that the text box is grey out, and you cannot write anymore because the websockets connection is failing.

Additional context
I suspect here you can't just return and ignore a new connection:

async def websocket_endpoint(websocket: WebSocket, user_id: str = "user"):
"""
Endpoint to handle incoming WebSocket connections by user id, process messages, and check for messages.
"""
# Retrieve the `ccat` instance from the application's state.
ccat = websocket.app.state.ccat
if user_id in manager.active_connections:
# Skip the coroutine if the same user is already connected via WebSocket.
return

@zioproto zioproto added the bug Something isn't working label Nov 13, 2023
@Pingdred
Copy link
Member

Have you tried in develop? A few days ago I added a ws exception in case two connections are opened with the same user id

@zioproto
Copy link
Contributor Author

Yes I tried the develop branch after PR #556

The error changed in the logs:

cheshire_cat_core           | [2023-11-15 15:38:40.948] ERROR  cat.routes.websocket..websocket_endpoint::94 => "A websocket connection with ID 'user' has already been opened."

But the buggy behaviour is exactly the same.

@Pingdred
Copy link
Member

Ok then it seems that the old websocket connection is not closed with the page refresh, so that the core refuse the new connection, can you check if after the refresh the in the cat's logs appears

cheshire_cat_core  | INFO:     connection closed

before (not necessarily in the previous line)

cheshire_cat_core | ERROR  cat.routes.websocket..websocket_endpoint::94 => "A websocket connection with ID 'user' has already been opened."

Thanks

@zioproto
Copy link
Contributor Author

Here the log snippet:

cheshire_cat_core           | INFO:     192.168.65.1:60896 - "GET /plugins/settings/ HTTP/1.1" 403 Forbidden
cheshire_cat_core           | INFO:     192.168.65.1:60893 - "GET /plugins/ HTTP/1.1" 403 Forbidden
cheshire_cat_core           | INFO:     192.168.65.1:60894 - "GET /memory/collections/ HTTP/1.1" 403 Forbidden
cheshire_cat_core           | [2023-11-15 15:38:40.948] ERROR  cat.routes.websocket..websocket_endpoint::94 => "A websocket connection with ID 'user' has already been opened."
cheshire_cat_core           | INFO:     ('192.168.65.1', 60897) - "WebSocket /ws/user" 403
cheshire_cat_core           | INFO:     connection failed (403 Forbidden)
cheshire_cat_core           | INFO:     192.168.65.1:60896 - "GET / HTTP/1.1" 200 OK
cheshire_cat_core           | [2023-11-15 15:38:40.966] INFO   cat.routes.websocket..websocket_endpoint::111 => 'WebSocket connection closed'
cheshire_cat_core           | [2023-11-15 15:38:40.980] INFO   cat.routes.websocket..websocket_endpoint::124 => "Removed WebSocket connection with ID 'user'"
cheshire_cat_core           | INFO:     connection closed
cheshire_cat_core           | INFO:     connection closed

@pieroit pieroit moved this to 📋 Backlog in Cat Project Kanban Apr 19, 2024
@pieroit pieroit closed this as completed Apr 21, 2024
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Cat Project Kanban Apr 21, 2024
@pieroit
Copy link
Member

pieroit commented Apr 21, 2024

Cannot reproduce this error anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

3 participants