NB2KG future websocket creation callback is causing WebSocketClosedError exception

[charlieeeeeee]

This issue is related to #14.

The issue is,

When websocket creation is slow. An WeSocketClosedError exception will occur.

This issue is induced by the callback NB2KG pass to the future for websocket creation, when the websocket is done. Future will call the callback that NB2KG registered to it. Here’s the code for registering the callback.

def on_open(self, kernel_id, message_callback, **kwargs):
        '''Web socket connection open.'''
        self._connect(kernel_id)
        loop = IOLoop.current()
        loop.add_future(
            self.ws_future,
            lambda future: self._read_messages(message_callback)
        )

the messsage_callback actually contains the write_message function from the base class WebSocketHandler

Here is the issue, according to the API reference, this callback will ONLY be called when the future completes. After adding debug lines, I found out that, the future actually completed with a websocket available to use (Because of issue #14 websocket creation will wait indefinitely). HOWEVER, the callback function we registered DOES NOT USE it, it uses the ws_connection parameter maintained by its own class. Observe the base class write_message function,

def write_message(self, message, binary=False):
        if self.ws_connection is None:
            raise WebSocketClosedError()
        if isinstance(message, dict):
            message = tornado.escape.json_encode(message)
        return self.ws_connection.write_message(message, binary=binary)

The base class has it’s own variable called ws_connection, which is used to make the message call. HOWEVER, that is not what the future just created. The self.ws NB2KG created is maintained inside the KernelGatewayWSClient class.

Either NB2KG needs to update the ws_connection to the self.ws that future just created

OR

The callback registered to future needs to use the client override method instead, instead of calling the super method, NB2KG need to call the self.client._write_message which is using the websocket future just created. Something like this,

def write_message(self, message):
        '''Send message back to client.'''
        _#super(WebSocketChannelsHandler, self).write_message(message)_
        self.client._write_message(message)

This way, upon future finish creating the websocket, it uses this callback, and uses the “client” write_message method which is using the self.ws which is now defined.

Will append log information later.

[kevin-bates]

Thanks @charlieeeeeee - this obviously took some time.
After spending the last couple days looking at this, I believe the issue is more tied to what the client application is doing in this case (I'll speak more on Issue #14). However, the code here does not do a very good job of protecting against this scenario and I'll be delivering a PR to address that.

The suggestion of replacing the call to the super class write_message() with self.client._write_message() is not correct since the super class corresponds to the websocket in use by the Notebook client, while the method on self.client corresponds the websocket to communicate to the Gateway Server. This is more evident when hitting NB2KG with an actual notebook instead of a program since the former requires you the output - but that will never happen since nothing gets returned if the methods are switched. As a result, one of the other areas addressed in my pending PR will be better variable names and comments - to better-distinguish targets.