Websocket subprotocol checked before the connection is established.

[Zsailer]

I'm seeing the following issue when launching (pending) kernels after #657

[E 2022-03-01 22:26:12.490 ServerApp] Uncaught exception in ZMQStream callback
    Traceback (most recent call last):
      File "/<user_path>/python3.8/site-packages/zmq/eventloop/zmqstream.py", line 431, in _run_callback
        callback(*args, **kwargs)
      File "/<user_path>/python3.8/site-packages/zmq/eventloop/zmqstream.py", line 189, in <lambda>
        self.on_recv(lambda msg: callback(self, msg), copy=copy)
      File "/<user_path>/python3.8/site-packages/jupyter_server/services/kernels/handlers.py", line 518, in _on_zmq_reply
        if self.selected_subprotocol == "v1.kernel.websocket.jupyter.org":
      File "/<user_path>/python3.8/site-packages/tornado/websocket.py", line 373, in selected_subprotocol
        assert self.ws_connection is not None
    AssertionError
[E 2022-03-01 22:26:12.491 ServerApp] Uncaught exception in zmqstream callback

It seems that ws_connection is not necessarily set before selected_subprotocol is checked in the _on_zmq_reply method.

Pinging @davidbrochart, in case you have any insights. I'll take a closer look later this week too.

[davidbrochart]

It's strange because we also access selected_subprotocol earlier in the same method, so I would expect the exception to be raised there too.
selected_subprotocol is a property that first checks if ws_connection is not None, and ws_connection is None if the Sec-WebSocket-Version is not in ("7", "8", "13"), so you could check the request headers for this field.
Tornado seems to only support these versions. What browser are you using?
In any case, we could try to access self.selected_subprotocol, and if it raises an error, fall back to the old protocol (maybe have our own property for that, so that we don't try each time we want to access it).

[Zsailer]

Thanks for tracking this down, @davidbrochart!

Tornado seems to only support these versions. What browser are you using?

Interesting! The case I shared above happened while using Safari 14.1.2. I'm not seeing this issue in other browsers or even other versions of Safari.

In any case, we could try to access self.selected_subprotocol, and if it raises an error, fall back to the old protocol (maybe have our own property for that, so that we don't try each time we want to access it).

I agree. We can even make the default subprotocol a configurable trait if there's demand for this.

[davidbrochart]

I agree. We can even make the default subprotocol a configurable trait if there's demand for this.

We can already configure the prefered kernel websocket protocol to use.
I'll open a PR to fix this issue.