sanic-org / sanic

Accelerate your web app development | Build fast. Run fast.
https://sanic.dev
MIT License
17.92k stars 1.54k forks source link

There is an obvious bug in ASGI WebsocketConnection of Sanic #2638

Closed piekey1994 closed 1 year ago

piekey1994 commented 1 year ago

Is there an existing issue for this?

Describe the bug

I started my sanic app with UvicornWorker. The original websocket will become WebsocketConnection. When I call the ws.recv function will report an error if bytes data is received at this time. KeyError:‘text’ https://github.com/sanic-org/sanic/blob/main/sanic/server/websockets/connection.py ` async def recv(self, *args, **kwargs) -> Optional[str]: message = await self._receive()

    if message["type"] == "websocket.receive":
        return message["text"]
    elif message["type"] == "websocket.disconnect":
        pass

    return None`

There is no data of bytes type processed here.

Code snippet

No response

Expected Behavior

No response

How do you run Sanic?

ASGI

Operating System

ubuntu

Sanic Version

22.3

Additional context

No response

ahopkins commented 1 year ago

I cannot reproduce this on main, or even on 22.3 using uvicorn.

from sanic import Request, Sanic

app = Sanic("TestApp")

@app.websocket("/feed")
async def feed(request: Request, ws):
    while True:
        data = await ws.recv()
        await ws.send(data)

I am closing this now as I see no bug. If you have a reproducible snippet, please send.

piekey1994 commented 1 year ago

I cannot reproduce this on main, or even on 22.3 using uvicorn.

from sanic import Request, Sanic

app = Sanic("TestApp")

@app.websocket("/feed")
async def feed(request: Request, ws):
    while True:
        data = await ws.recv()
        await ws.send(data)

I am closing this now as I see no bug. If you have a reproducible snippet, please send.

Is there an existing issue for this?

Describe the bug

I started my sanic app with UvicornWorker. The original websocket will become WebsocketConnection. When I call the ws.recv function will report an error if bytes data is received at this time. KeyError:‘text’ https://github.com/sanic-org/sanic/blob/main/sanic/server/websockets/connection.py ` async def recv(self, *args, **kwargs) -> Optional[str]: message = await self._receive()

    if message["type"] == "websocket.receive":
        return message["text"]
    elif message["type"] == "websocket.disconnect":
        pass

    return None`

There is no data of bytes type processed here.

Code snippet

No response

Expected Behavior

No response

How do you run Sanic?

ASGI

Operating System

ubuntu

Sanic Version

22.3

Additional context

No response

I cannot reproduce this on main, or even on 22.3 using uvicorn.

from sanic import Request, Sanic

app = Sanic("TestApp")

@app.websocket("/feed")
async def feed(request: Request, ws):
    while True:
        data = await ws.recv()
        await ws.send(data)

I am closing this now as I see no bug. If you have a reproducible snippet, please send.

main.py

from sanic import Sanic

app = Sanic('ws')

@app.websocket('/feed')
async def feed(req,ws):
    while True:
        data =await ws.recv()
        print(data)
        if data is None:
            break

if __name__ == '__main__':
    import websockets
    import asyncio
    async def main():
        async with websockets.connect('ws://127.0.0.1::8889/feed') as ws:
            await ws.send('123')
            await ws.send(bytes(123))
    asyncio.get_event_loop().run_until_complete(main())

run: gunicorn main:app --bind 0.0.0.0:8889 --worker-class uvicorn.workers.UvicornWorker

ahopkins commented 1 year ago

Nope. Still works fine for me.

piekey1994 commented 1 year ago

Nope. Still works fine for me.

That's weird. However, you can look at this code in sanic, and it does seem to have a problem. It seems to only handle text, right? connection.py

    async def recv(self, *args, **kwargs) -> Optional[str]:
        message = await self._receive()

        if message["type"] == "websocket.receive":
            return message["text"]
        elif message["type"] == "websocket.disconnect":
            pass

        return None
ahopkins commented 1 year ago

Yeah, will go back and look at the spec to see exactly what it's supposed to be.