Websockets are automatically closed after response

[Marco-Sulla]

Steps to reproduce:

1. Install aiohttp>=0.15.0
2. Create server.py with this code:

import asyncio
from aiohttp import web


@asyncio.coroutine
def handle(request):
    name = request.match_info.get('name', "Anonymous")
    text = "Hello, " + name
    return web.Response(body=text.encode('utf-8'))


def wshandler(request):

    msg = b"Hello"

    ws = web.WebSocketResponse()

    print("before start")
    yield from ws.prepare(request)
    print("after start")

    ws.send_bytes(msg)

    return ws


@asyncio.coroutine
def init(loop):
    app = web.Application(loop=loop)
    app.router.add_route('GET', '/echo', wshandler)
    app.router.add_route('GET', '/{name}', handle)

    srv = yield from loop.create_server(app.make_handler(),
                                        '127.0.0.1', 8088)
    print("Server started at http://127.0.0.1:8088")
    return srv

loop = asyncio.get_event_loop()
loop.run_until_complete(init(loop))
loop.run_forever()

Substitute yield from ws.prepare(request) with ws.start(request) for older versions.

1. Run the server
2. Open your browser at http://127.0.0.1:8088/Marco
3. Open the browser console
4. Paste this code:

ws = new WebSocket("ws://127.0.0.1:8088/echo");
ws.binaryType = "arraybuffer";

ws.onopen = function (e) {
    console.log("WEBSOCKET OPENED")
}

ws.onclose = function (e) {
    console.log("WEBSOCKET CLOSED")
}

ws.onmessage = function (e) {
  var msg_view = new DataView(e.data);
  var msg_decoder = new TextDecoder("utf-8");
  var msg = msg_decoder.decode(msg_view);
  console.log(msg);
}

ws.onerror = function (e) {
    console.log("SOMETHING WRONG WITH WEBSOCKET")
}

RESULT:

In console is logged:

WEBSOCKET OPENED
Hello
WEBSOCKET CLOSED

EXPECTED:

In console should be logged

WEBSOCKET OPENED
Hello

[redixin]

wshandler is a coroutine, and websocket is closed as soon as this coroutine is finished.

It is intended to use in loop. Something like this:

    while True:
        data = yield from ws.receive()
        ws.send_str(data)

[Marco-Sulla]

So I can't use the websocket outside that coroutine? Quite unhandy.

[asvetlov]

Strictly speaking you should use the single task for receiving data from websocket.
ws.send_*() are not coroutines, you may cal it from everywhere (but websocket should be nonclosed, obviously).

[fafhrd91]

both client and web implementation are higher level api on top of aiohttp.websoket implementation,
so you can implement your own high level api.

On Dec 17, 2015, at 9:23 AM, MarcoSulla notifications@github.com wrote:

So I can't use the websocket outside that coroutine? Quite unhandy.

—
Reply to this email directly or view it on GitHub #691 (comment).

[Marco-Sulla]

It seems to me counterintuitive to have a loop only to maintain the websocket opened, since they are usually created to have a persistent connection channel between client and server, and not a disposable one. What's the advantage from 0.14.4?

[redixin]

It is possible to use synchronous approach instead of callbacks like twisted and friends. That's why yield from was introduced.

[Marco-Sulla]

I can be wrong, but I think you could do this also before. The only difference is that if the coroutine exists, the websocket is closed, even if the object still exists.
Indeed I'm working on a code that adds the websockets to a list, that lives outside the coroutine. So even if the coroutine exits, the websocket object still exists, and it's highly counterintuitive that it's closed only because the original coroutine is finished.

Please see the modified example:

import asyncio
from aiohttp import web


a = []


@asyncio.coroutine
def handle(request):
    name = request.match_info.get('name', "Anonymous")
    text = "Hello, " + name
    return web.Response(body=text.encode('utf-8'))


def wshandler(request):

    msg = "Hello"

    ws = web.WebSocketResponse()

    print("before start")
    ws.start(request)
    print("after start")

    ws.send_str(msg)

    a.append(ws)

    return ws


@asyncio.coroutine
def init(loop):
    app = web.Application(loop=loop)
    app.router.add_route('GET', '/echo', wshandler)
    app.router.add_route('GET', '/{name}', handle)

    srv = yield from loop.create_server(app.make_handler(),
                                        '127.0.0.1', 8088)
    print("Server started at http://127.0.0.1:8088")
    return srv

loop = asyncio.get_event_loop()
loop.run_until_complete(init(loop))
loop.run_forever()

Ok, it's not good to have a global, but do you see the problem? The object still live, so why it must be closed?

[asvetlov]

Mentally replace web.WebSocketResponse with regular web.Response.

Is it intuitive that response is finished and should not be used after finishing web-handler even if you have stored it in global list?
Response has web-handler scope, websocket response is not an exception.

[redixin]

This all is to be able to work in synchronous manner:

def wshandler(request):
    msg = "Hello"
    ws = web.WebSocketResponse()
    print("before start")
    ws.start(request)
    ws.send_str(msg)
    a.append(ws)
    while True:
        try:
            msg = yield from ws.receive()
        except aiohttp.errors.ClientDisconnectedError:
            LOG.info("Disconnected %s" % ws)
        if msg.tp == web.MsgType.close:
            break
    a.remove(ws)  # <- see
    return ws

As for me, using protocol instances with all this methods like connection_made, connection_lost, data_received, etc is good, and I'm ok with both approaches.

It is not better or worst, it just another.

[Marco-Sulla]

@asvetlov: because "special cases aren't special enough to break the rules"? The problem is you're reasoning in terms of http connection. Websocket is a different protocol, and its connection is intended to be persistent, more similar to "classical" sockets. Indeed you have not to prepare() a "normal" response object.

The other problem is naming it "reponse". That object is not a response, IMHO. Websockets are a persistent full-duplex connection, so the same connection can be used to get "requests" and send "responses" until not explicitly closed or garbage-collected because not used anymore. See WHATWG spec: https://html.spec.whatwg.org/multipage/comms.html#network

Lastly, the way you're implementing it impose people to listen for client messages. But in theory I could use websockets only for sending messages to the client. By the way, that's what the code I'm working on is actually doing. The fact you have to do this or die it's not only counterintuitive, it's cucumbersome.

I can live with it and code this way, since I do not have to work with aiohttp very much. But I strongly encourage you to rethink about your decision.

[asvetlov]

@MarcoSulla that ship has sailed.
You may try to make different implementation though.

BTW

1. You have to call prepare() for streaming responses manually.
2. People should listen for incoming events. Even if browser doesn't send and client messages it may send system ones like pings and close notifications.