close_code and close_reason are not set for WebSocketHandler

[vovanec]

Hey,

I've just noticed that close_code and close_reason are never set for WebSocketHandler, so they're always None. In my custom websocket handler implementation I'm overriding that by overriding close method in a way:

    def __init__(self, handler, mask_outgoing=False):
        self.closed = False

    def close(self, code=None, reason=None):
        """Closes the WebSocket connection."""

        if not self.closed:
            super().close(code, reason)
            self.handler.close_code = code
            self.handler.close_reason = reason
            self.closed = True

I'm happy to submit pull request but not really sure whether the way I'm working it around is completely correct.

Please advise. --Vovan

[vovanec]

I've introduced self.closed = False flag to the WebSocketProtocol13 as I see that close() method called two times - first when I'm explicitly calling close() on handler, the second time - from on_connection_close() callback in WebSocketProtocol base class:

class WebSocketProtocol(object):
    """Base class for WebSocket protocol versions.
    """

    def on_connection_close(self):
        self._abort()

Here, self._abort() calls to self.close() with no parameters, so any close_code/reason I've set before simply reset.

[bdarnell]

This is by design: the parameters to close() are the code and reason you want to send to the other side when closing the connection; the close_code and close_reason attributes are the values the client supplied when closing the connection.

[krkd]

According to your comment close should be called with codes that it recieved. but according to https://github.com/tornadoweb/tornado/blob/master/tornado/websocket.py#L838 Connection will store close code and reason but it want passs it to close.

[bdarnell]

There are two close codes and reasons: the client and the server can each send their own close code. To send a close code, pass it to WebSocketHandler.close(); to receive one override WebSocketHandler.on_close() and examine self.close_code. Do not attempt to override WebSocketHandler.close().

At line 838, self is a WebSocketProtocol13, not a WebSocketHandler. This is an object that application code should not interact with; it shouldn't matter to the application what is passed to WebSocketProtocol13.close.

[krkd]

So if client call close with some code tornado will send empty close frame, which treated on client as 1005 (no status). is that expected behavior? 29.04.2015 4:24 пользователь "Ben Darnell" notifications@github.com написал:

There are two close codes and reasons: the client and the server can each send their own close code. To send a close code, pass it to WebSocketHandler.close(); to receive one override WebSocketHandler.on_close() and examine self.close_code. Do not attempt to override WebSocketHandler.close().

At line 838, self is a WebSocketProtocol13, not a WebSocketHandler. This is an object that application code should not interact with; it shouldn't matter to the application what is passed to WebSocketProtocol13.close.

— Reply to this email directly or view it on GitHub https://github.com/tornadoweb/tornado/issues/1377#issuecomment-97279899.

[bdarnell]

There is no guarantee that the two sides of the connection send the same close code (RFC 6455 section 7.1.5, so it is expected that sometimes you will receive a different close code than the one you sent.

However, section 5.5.1 says that applications typically echo the response code they receive (but not the reason) when acknowledging a close frame. This is surprising to me, but it's in the spec so line 838 should be updated to pass self.close_code to self.close().

[desertkun]

This happens to the client version, WebSocketClientConnection – close_code and close_reason is always None. According to this line, it never appears to be filled.

[bdarnell]

They're initialized to None at the line that you linked, but they are still filled in when the server closes the connection. This is tested here: https://github.com/tornadoweb/tornado/blob/50f9eedd89f26fc86974461b6ddd84343504be93/tornado/test/websocket_test.py#L225-L231