encode / uvicorn

An ASGI web server, for Python. 🦄
https://www.uvicorn.org/
BSD 3-Clause "New" or "Revised" License
8.37k stars 722 forks source link

Autoclosing behavior for websockets #199

Closed almarklein closed 5 years ago

almarklein commented 5 years ago

Experimenting with websockets, it seems that uvicorn automatically closes the websocket as soon as the task that initiated the websocket is done. But what if I want to keep my websocket somewhere, and e.g. use it from other tasks, e.g. one task that keeps listening for incoming messages, and other tasks that occasionally send messages?

almarklein commented 5 years ago

Or maybe ... I should let that task wait for the ws to close?

tomchristie commented 5 years ago

Or maybe ... I should let that task wait for the ws to close?

Indeed, yes.

The main task that runs should supervise any child tasks, and should not complete until all child tasks have completed. See https://github.com/django/asgiref/issues/65 and https://github.com/django/asgiref/commit/9d56198087ef2de6a16ff925e1dff6583f1eac13

Also: If there are child tasks against a websocket connection, then the typical pattern should be for the parent to listen for the disconnect, and cancel any child tasks if/when it occurs.

(Doing anything else would lead to resource leaks and underdetermined shutdown behaviors.)

tomchristie commented 5 years ago

Related: https://github.com/encode/starlette/issues/58#issuecomment-424257205

almarklein commented 5 years ago

Thanks!

vlori2k commented 4 years ago

COULD SOMEONE HELP? Im still having this issue:

"RuntimeError: Unexpected ASGI message 'websocket.send', after sending 'websocket.close'."

This happens when a client sends something different/or another message then JSON.

CAN SOMEBODY HELP?

almarklein commented 4 years ago

@vlori2k When the task that handles a websocket ends, this is considered the end of the lifetime of the websocket. So if you want to keep using the websocket ... your coroutine should simply not end. Another thing to consider is that you will want to keep receiving from the websocket to detect it being closed by the client. Combine the two: let your coroutine receive messages until it receives a disconnect. Sending messages can be done from the same task (e.g. upon receiving a message), or by another task.

(With task I mean an asynctio-task.)

What that looks like depends on the framework you're using. In Asgineer this could look like e.g. this echo example, or this chat example.

vlori2k commented 4 years ago

@almarklein my concern is still: is my code correct or not? as long as the client IS connected and also send CORRECT json, he remains connected to the server, but if he sends a plain text or a wrong json he automatically gets disconnected.

Could you test this code at your place and give feedback pls?

my code is here:

import socketio
from fastapi import FastAPI, WebSocket
from starlette.websockets import WebSocketDisconnect

from starlette.exceptions import HTTPException

from typing import List

app = FastAPI()

sio = socketio.AsyncServer(async_mode='asgi')
socket_app = socketio.ASGIApp(sio, static_files={'/': 'app.html'})
background_task_started = False

how_many_clients_are_connected = []

class ConnectionManager:
    def __init__(self):
        self.active_connections: List[WebSocket] = []

    async def connect(self, websocket: WebSocket):
        await websocket.accept()
        self.active_connections.append(websocket)

    async def disconnect(self, websocket: WebSocket):
        await websocket.close(code=1000)
        self.active_connections.remove(websocket)
        print("STATUS : There are currently {} " .format(len(how_many_clients_are_connected)) + "clients connected to server")
        print("STATUS : There are currently {} ".format(len(manager.active_connections)) + "connections ongoing!")

    async def send_personal_message(self, message: str, websocket: WebSocket):
        await websocket.send_text(message)

    async def broadcast(self, message: str):
        for connection in self.active_connections:
            await connection.send_text(message)

    async def on_receive(self, data):
        """Override to handle an incoming websocket message"""
        await self.websocket.receive(data)

manager = ConnectionManager()

import json
@app.websocket("/ws/{client_id}")
async def websocket_endpoint(websocket: WebSocket, client_id: int):
    for user_IDS_that_are_connected in how_many_clients_are_connected:
        if user_IDS_that_are_connected == client_id:

            print("ID is already in use! User not allowed")
            await manager.broadcast(f"Somebody tried to connect with the same ID as client: {client_id}")
            raise HTTPException(status_code=500, detail="Nope! I don't like 3.")

            #return False
    else:
        await manager.connect(websocket)
        how_many_clients_are_connected.append(client_id)

        try:
            print("STATUS : There are currently {} " .format(len(how_many_clients_are_connected)) + "clients connected to server")
            print("STATUS : There are currently {} ".format(len(manager.active_connections)) + "connections ongoing!")

            for client_numbers in how_many_clients_are_connected:
                print("Client IDs: {} " .format(client_numbers))

            while True:
                try:
                    data = await websocket.receive_json()
                    print(data)
                except json.JSONDecodeError:
                    await manager.send_personal_message("You have not sent JSON object correctly.. fix it plz", websocket) # NOW THE CLIENT GETS DISCONNECTED AUTOMATICALLY, WHY??

        except WebSocketDisconnect:
            print(f"Client #{client_id} left the chat")
            how_many_clients_are_connected.remove(client_id)
            await manager.disconnect(websocket)
            await manager.broadcast(f"Client #{client_id} left the chat")

app.mount('/', socket_app)
almarklein commented 4 years ago

That looks fine on first glance.

... but if he sends a plain text or a wrong json he automatically gets disconnected.

I'd think that the except json.JSONDecodeError takes care of that, does it not?

vlori2k commented 4 years ago

Nvm. It is working now!! Thank you for answer anyways.

So according to you, my code looks fine? You have some suggestion on how i can improve it?

almarklein commented 4 years ago

Well, I'm not familiar with FastAPI tbh, and I am not going to do a full code review :) but I could not detect anything wrong with it. The pattern to keep calling receive_json in a loop is the way to go, I think.