Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems after upgrading to latest tornado and sockjs-tornado #67

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dimabxsx
Copy link

After switching to the latest tornado and sockjs-tornado stuck with this exceptions.

First one comes from SockJSWebSocketHandler._execute

e:net05 140923 08:51:07 [http1connection:53] Uncaught exception
Traceback (most recent call last):
  File "/opt/.env/lib/python2.7/site-packages/tornado/http1connection.py", line 234, in _read_message
    delegate.finish()
  File "/opt/.env/lib/python2.7/site-packages/tornado/httpserver.py", line 282, in finish
    self.delegate.finish()
  File "/opt/.env/lib/python2.7/site-packages/tornado/web.py", line 1884, in finish
    self.execute()
  File "/opt/.env/lib/python2.7/site-packages/tornado/web.py", line 1914, in execute
    self.handler._execute(transforms, *self.path_args, **self.path_kwargs)
  File "/opt/.env/lib/python2.7/site-packages/sockjs/tornado/websocket.py", line 44, in _execute
    self.stream.write(escape.utf8(
AttributeError: 'NoneType' object has no attribute 'write'

Can't reproduce this. Checking self.stream value before write and close do the job, but i'm not sure it's a good solution.

Second one is related to xhr-polling transport.

e:net04 140923 09:17:07 [__init__:170] Tried to write more data than Content-Length
Traceback (most recent call last):
  File "/opt/server/net/__init__.py", line 150, in _net_message_handler
    h(*msg['args'], **msg['kwargs'])
  File "/opt/server/net/handler/base.py", line 120, in close_connect
    self.close()
  File "/opt/.env/lib/python2.7/site-packages/sockjs/tornado/conn.py", line 65, in close
    self.session.close()
  File "/opt/.env/lib/python2.7/site-packages/sockjs/tornado/session.py", line 381, in close
    self.handler.send_pack(proto.disconnect(code, message))
  File "/opt/.env/lib/python2.7/site-packages/sockjs/tornado/transports/xhr.py", line 52, in send_pack
    self.flush(callback=self.send_complete)
  File "/opt/.env/lib/python2.7/site-packages/tornado/web.py", line 855, in flush
    return self.request.connection.write(chunk, callback=callback)
  File "/opt/.env/lib/python2.7/site-packages/tornado/http1connection.py", line 418, in write
    self._pending_write = self.stream.write(self._format_chunk(chunk))
  File "/opt/.env/lib/python2.7/site-packages/tornado/http1connection.py", line 394, in _format_chunk
    "Tried to write more data than Content-Length")
HTTPOutputError: Tried to write more data than Content-Length

Caused by calling SockJSConnection.close when using xhr-polling.

Can be reproduced in chat example.

  1. Add self.close() call somewhere in ChatConnection.on_message
  2. Disable all transports except xhr-polling

When close will be called, you'll get same exception.

@maparent
Copy link
Contributor

First issue probably solved by 7153769.

@dimabxsx
Copy link
Author

No, problem with websockets still not solved.It can be reproduced by sending unexpected values in Connection or Upgrade headers.
I ended up with removing SockJSWebSocketHandler._execute at all.

By the way, is there any reasons to override this method? Both Connection and Upgrade headers will be checked in tornado's WebSocketHandler.get itself. Allowed methods can de defined in web.RequestHandler.SUPPORTED_METHODS.

@sarasafavi
Copy link

Can confirm the bug in issue 1 above still exists for anyone using Tornado >=4.0.

Unfortunately I think @dimabxsx's proposal is a breaking change for anyone still using 3.2 (or lower, god forbid...). The checks for Connection & Upgrade moved from WebSocketHandler._execute to a new WebSocketHandler.get in 4.0 (and likewise _execute was eliminated completely) -- ref: tornadoweb/tornado@c8ce451.

I don't know of a clean way of supporting both versions, so this may just mean a hard dependency on 3.2 here.

@mrjoes
Copy link
Owner

mrjoes commented Apr 16, 2015

Only reason why _execute() was overridden - there are client-side SockJS tests and sockjs-tornado had to be compliant. Older Tornado versions didn't have this checking logic in place. _execute should stay for backwards compatibility, but can check if connection is still there and just exit immediately.

Second issue - not sure what's going on. Will try to troubleshoot. Most likely caused by some additional checks in Tornado or sockjs-tornado to send something to the client on closed connection.

@timdawborn
Copy link
Contributor

The second issue is caused by closing the connection from within on_open or on_message by calling self.close() (we had the same issue). It can be fixed by closing the connection in the next async loop allowing the current loop to finish what it's doing.

def do_close(self):
    self.close()

def on_open(self, data):
    ...
    IOLoop.current().add_callback(self.do_close)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants