Is autobahn compatible with twisted 16.3.0?

[tardyp]

Since twisted 16.3.0, buildbot's websocket ceased to function.

after bisect, it looks like related to commit 2d39ccad9d8a9b86229dd7fb1ce47ca841b730be from twisted
which is related to this comment in release note:

Deprecations and Removals

• twisted.web would previously dispatch pipelined requests
  simultaneously and queue the responses. This behaviour did not
  enforce any of the guarantees required by RFC 7230 or make it
  possible for users to enforce those requirements. For this reason,
  the parallel dispatch of requests has been removed. Pipelined
  requests are now processed serially. (#8320)

Are you testing crossbar.io with latest twisted? do you have this issue, or is this related to the way buildbot integrates autobahn?

[meejah]

See also: magic-wormhole/magic-wormhole#62

[meejah]

@tardyp I have run some quick tests against 16.3.0 on Debian and it seemed to work fine (with master) but I am running some more tests now. Given that wormhole is seeing the same thing, it's likely a bug -- could be websockets-only (i.e. without WAMP over top)?

[oberstet]

@tardyp 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": #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 ..

[tardyp]

Indeed, we are using Twisted web.

I dont think this is about http/2. I have the same bisect commit as @warner founds: twisted/twisted@2d39cca

Which is not about http/2, but rather about parallel http/1.1 pipelining

On the browser, I see the websocket connection setup, the response, and upgrade to ws, but I dont see websocket onopen on the JS.

On the first I tried to reverse to older version of autobahn, which did not resolve the issue.
So if it is a bug of autobahn that is triggered by tw16.3.0 , it has been present from long.

[glyph]

@tardyp - The change in question (removal of HTTP/1.1 pipelining) was to facilitate a compatible HTTP/2 layer, which is what I think @oberstet was referring to.

[glyph]

I am very curious how this broke autobahn, since there was quite a bit of discussion about how to do this in the lowest-impact way possible, and we thought the strategy that we took here should have actually been more friendly to things like autobahn that want to steal the transport, not less. Of course any change in behavior is a risk but it would be helpful to know what WebSocketResource was doing that ultimately conflicted with this change.

[glyph]

Paging @hawkowl to the thread…

[oberstet]

Yes, @glyph is right about what I meant. Plus I am puzzled too about exactly how this broke things in Autobahn.

Looking into history, I am also wondering why line 185 in the following has been removed f91bbab#diff-f65d4b57081aec20e67cd4121dbf9183

The code in above right now isn't symmetric between Py 2/3 anymore .. where it only would need to account for bytes vs strings ..

In any case: Autobahn's WebSocket resource for Twisted Web has always been kinda of a hack .. in a way, I knew it would come back biting sooner or later;)

[tomprince]

It looks like that commit makes twisted pause the transport, and since the transport gets stolen out from under twisted, nothing ever unpauses it.

[meejah]

I can confirm I've repeated this (on both master and v0.14.1) and that @tomprince's patch works.

[glyph]

Woohoo, thanks @tomprince ! This also points to a potential fix on the Twisted side, if other users have problems with this change: buffer incoming request but don't dispatch them to be processed, as opposed to pausing the transport so new ones won't arrive...

[tomprince]

@glyph I don't think that would fix it, since the buffered data would still need to be pushed to whoever is stealing the transport. That'd actually be a problem with this fix, if the underlying transport isn't an IPushProducer.

[oddjobz]

Just a note, the broken 16.3.0 is still being served up by "pip" ...

[tomprince]

Just a note, the broken 16.3.0 is still being served up by "pip"

I'm not sure it is entirely fair to say that 16.3.0 is broken. There is just one particular case, when autobahn is stealing an object out from under twisted, and 16.3.0 happened to change the state that object was in when it got stolen. There is a fix in autobahn (#705) that is backwards compatible with old Twisted versions and an old twisted issue for providing a documented way of stealing the transport.

[oddjobz]

Sure, to reword;

Just a note, the version of 16.3.0 involved in the issue that causes Crossbar to fail silently, is still being served up by pip ... and it would be really cool if the patch that apparently already exists could make it's way to the pip repositories so more dumb programmers like me don't lose all their hair at once ... ;)

[oberstet]

@oddjobz things are fixed/worked around in AutobahnPython/Crossbar.io master branches now (it works with Twisted 16.3) - we'll be doing pip releases soonish ..

[oddjobz]

:) .. something doesn't feel quite right with the way that PIP is behaving with regards to dependencies, i.e. that by default it will install the latest version of any given component. Given maintainers of dependent packages will not test their updates with all packages that rely on "them", this is essentially inserting an untested code combination into a live ecosystem. In this instance, once 16.3 was up, anyone installing Crossbar from scratch was going to get an untested combination of Crossbar + Twisted 16.3 which in this case caused an issue. Is there any way in PIP to tag dependencies to particular versions, so that when you install a package (like Crossbar) it will force versions of dependencies that have been tested and are known to work with the given package? i.e. in this particular example, would have forced twisted back to 16.2?

[meejah]

@oddjobz yes, it's possible to pin versions (and/or use <= some version). However, pip doesn't have a "real" dependency-resolver :(

The usual advice I've seen is to pin versions for "application-like" things, and leave them open-ended for "library-like" things to make installs more-likely to play nicely (e.g. if Autobahn pinned to Twisted==16.0.0 but some other library pinned to Twisted==something_else). pip just installs the first one it sees...(i.e. if the first dependency it finds that mentions Twisted asks for 14.0.0, that's what it'll install even if a later one asks for 16.1.0).

So, it may indeed make sense to pin exact, tested versions for Crossbar -- but I don't think it makes sense to pin versions for the Autobahn Python library itself.

[oberstet]

Yep. Pinning libraries (like AB) is I guess a less good idea. For CB binary packages, we do pin already: https://github.com/crossbario/crossbar/blob/master/requirements.txt

Am 25.07.2016 9:16 nachm. schrieb meejah notifications@github.com:

@oddjobzhttps://github.com/oddjobz yes, it's possible to pin versions (and/or use <= some version). However, pip doesn't have a "real" dependency-resolver :(

The usual advice I've seen is to pin versions for "application-like" things, and leave them open-ended for "library-like" things to make installs more-likely to play nicely (e.g. if Autobahn pinned to Twisted==16.0.0 but some other library pinned to Twisted==something_else). pip just installs the first one it sees...(i.e. if the first dependency it finds that mentions Twisted asks for 14.0.0, that's what it'll install even if a later one asks for 16.1.0).

So, it may indeed make sense to pin exact, tested versions for Crossbar -- but I don't think it makes sense to pin versions for the Autobahn Python library itself.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//issues/701#issuecomment-235054937, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAOPfCfPQzfl0Uj3PqGcaYSGQLWGNzGtks5qZQt0gaJpZM4JHQGp.