tests hang against twisted-16.3.0

[warner]

Running 'tox' on wormhole trunk (currently at cdb5c19) hangs at test_scripts.Cleanup. Downgrading Twisted from current 16.3.0 to 16.2.0 lets it pass all tests.

[warner]

git bisect (awesome as ever) reveals that the problem started when Twisted landed twisted/twisted@1d69e04, to resolve Twisted#8320 "Remove pipelining from HTTPChannel". The ticket mentions something about it maybe breaking tests which might be using StringTransport.. sounds like we should investigate whether the wormhole tests were using those.

[warner]

@meejah hey, can you reproduce this? It doesn't seem to be affecting normal operation, but tests are messed up and I'm wondering if other Autobahn projects are seeing something similar (I don't see any autobahn tickets filed yet, so I'm wondering if our tests are doing something unusual).

So far it looks like the rendezvous server is receiving both websocket connections (onOpen) during the test, but it's not receiving any of the onMessage() that it's supposed to get. There are no exceptions in the logs, alas.

[meejah]

Is it Cleanup.test_text that hangs? If so, then "yes", I can repeat this with 16.3.0

Don't recall seeing this in Autobahn; will check there (we definitely use StringTransport for some tests). I do recall a lot of discussion on #twisted surrounding transports and HTTP (and HTTP/2) and I know Autobahn is one of the "less normal" cases that "does stuff" to the transport (e.g. so you can serve a WebSocket and a normal Web page from the same listener -- Autobahn gives you a help page if you HTTP GET to a WebSocket endpoint).

[meejah]

p.s. if you want debug logging from Autobahn, you can put a snippet like this in some test_*.py:

import txaio
txaio.start_logging(level='debug')

[meejah]

FWIW, I just confirmed that all Autobahn tests pass against 16.3.0 -- but, there isn't 100% coverage so could still be an Autobahn issue and not wormhole...

[oberstet]

@warner Are you using autobahn.twisted.resource.WebSocketResource (and hence running under Twisted Web)? My best guess: it will work when running WebSocket not under Twisted Web. It might be fallout from Twisted trying to support HTTP/2. For Autobahn, we plan to "reverse": crossbario/autobahn-python#641 (comment) : that is, parse the initial HTTP request ourselves, and only when the request isn't for an URL/Upgrade header to WebSocket, hand over to Twisted Web. That would shield us from the follies/fallout of HTTP/2 ..

[hawkowl]

This has affected Daphne as well.

We changed everything to go through the channel (self.channel.transport) so I think it is a side effect of that. I'll check.

[warner]

Oh, yes, the wormhole server is using WebSocketResource. The test spin up a new server (under the tox-installed local Twisted), so that's probably why the tests are failing. I haven't updated the real server in a few weeks: it must still be using Twisted-16.2.0, which is why normal operation is still working (clients use a non-twisted.web websocket). I bet if I upgraded that, normal wormhole usage would fail too.

[hawkowl]

I have come up with a reason -- the git commit you bisected added flow control to Request, meaning that the transport was paused (this is how we're handling pipelined requests now).

[hawkowl]

Bug fix written, making a PR on Autobahn....

[hawkowl]

Seems like @tomprince also fixed it :)

[meejah]

For bystanders reading this, you can install specifically Twisted 16.1.1 to work around this for now. That's pip uninstall twisted and then pip install twisted==16.1.1.

[meejah]

See also crossbario/autobahn-python#641

[oberstet]

@warner @hawkowl 's patch is on Autobahn master - so it should work with latest Twisted for you again ..

[warner]

I think there's been a new Autobahn release since this problem, and it's not happening on an updated tox just now, so it's probably safe to close this now.