Websocket client exception on redirect

[willmcgugan]

When the websocket client receives a redirect status code, rather than a 101, it throws the following exception:

ERROR:tornado.application:Uncaught exception
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/tornado/http1connection.py", line 237, in _read_message
    delegate.finish()
  File "/usr/local/lib/python3.6/site-packages/tornado/http1connection.py", line 674, in finish
    return self._delegate.finish()
  File "/usr/local/lib/python3.6/site-packages/tornado/simple_httpclient.py", line 500, in finish
    self.client.fetch(new_request, final_callback)
AttributeError: 'NoneType' object has no attribute 'fetch'
ERROR:tornado.application:Exception in callback functools.partial(<function wrap.<locals>.wrapped at 0x107cd9ea0>, <Future finished exception=_QuietException()>)
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/tornado/http1connection.py", line 237, in _read_message
    delegate.finish()
  File "/usr/local/lib/python3.6/site-packages/tornado/http1connection.py", line 674, in finish
    return self._delegate.finish()
  File "/usr/local/lib/python3.6/site-packages/tornado/simple_httpclient.py", line 500, in finish
    self.client.fetch(new_request, final_callback)
AttributeError: 'NoneType' object has no attribute 'fetch'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/tornado/ioloop.py", line 759, in _run_callback
    ret = callback()
  File "/usr/local/lib/python3.6/site-packages/tornado/stack_context.py", line 344, in wrapped
    raise_exc_info(exc)
  File "<string>", line 4, in raise_exc_info
  File "/usr/local/lib/python3.6/site-packages/tornado/stack_context.py", line 315, in wrapped
    ret = fn(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/tornado/simple_httpclient.py", line 395, in <lambda>
    lambda f: f.result())
  File "/usr/local/lib/python3.6/site-packages/tornado/gen.py", line 1113, in run
    yielded = self.gen.send(value)
  File "/usr/local/lib/python3.6/site-packages/tornado/http1connection.py", line 237, in _read_message
    delegate.finish()
  File "/usr/local/lib/python3.6/site-packages/tornado/http1connection.py", line 55, in __exit__
    raise _QuietException
tornado.http1connection._QuietException

The Websocket spec has this to say about redirects:

1. If the status code received from the server is not 101, the
   client handles the response per HTTP [RFC2616] procedures. In
   particular, the client might perform authentication if it
   receives a 401 status code; the server might redirect the client
   using a 3xx status code (but clients are not required to follow
   them), etc. Otherwise, proceed as follows.

So redirects don't appear to a requirement as such, but I don't see any reason why it shouldn't follow them. Websocket redirects have much the same use case as any http redirect. But the client should at the very least be able to handle a non-101 gracefully.

From what I've seen of the code, it looks like Tornado is at least trying to follow the redirect. So I'm hoping it won't be too much effort to fix. With a few pointers I may be able to tackle this myself. Any assistance would be appreciated.

[bdarnell]

Hmm, this is messy. websocket_connect tries to jump into the HTTP stack in the middle so it doesn't have the full AsyncHTTPClient available.

There are a few ways this could be fixed:

• Use a real AsyncHTTPClient instead of jumping straight into a Connection. I'm not sure what this would involve.
• Make a fake WebSocketHTTPClient that just has a fetch() method (to call back into a new websocket connection). I'm not sure whether this would be easier or harder than the first option.
• Raise an error in WebSocketClientConnection.headers_received, then catch that exception in websocket_connect and try the new URL. This is probably the simplest option (and the first step, of raising an exception in headers_received, is useful on its own to make this error cleaner).

[lavneetpuri]

I was configuring jupyter remote kernels through a gateway server authenticated using a single-sign-on redirection scheme when this exception was thrown. I must say it definitely took longer that expected to debug. The culprit was of course this line where the client arg for websocket handshake http-connection is passed as None.

Would there be any significant repercussions if an instance of simple_httpclient.SimpleAsyncHTTPClient was passed instead? An obvious one is the overhead of instantiating this class on every normal websocket connection request - which can be resolved by adding a follow_redirects: bool = False param to websocket_connect.

[vivamexico]

In my case, I don't need the websocket to follow the redirects, but I would like to be able to handle the exception. Unless and until websocket_connect takes a follow_redirects param, would it make sense to just set request.follow_redirects to False in websocket_connect? Then I can catch and handle tornado.httpclient.HTTPClientError.

My scenario is that I want to gracefully handle an situation where the websocket server is broken and continuously redirecting. The redirect is bogus, but I want to be able to detect the situation and retry the connection later.

[vivamexico]

I opened a pull request for the above mentioned change. Working on 5.1 branch (waiting for socket-js compatibility before upgrading to 6)