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

Breaking change in Websocket 6.0 release #1264

Closed
jamesstidard opened this issue Jul 18, 2018 · 20 comments
Closed

Breaking change in Websocket 6.0 release #1264

jamesstidard opened this issue Jul 18, 2018 · 20 comments
Milestone

Comments

@jamesstidard
Copy link
Contributor

Hi,

Just pipenv installed sanic and it pulled down the new websocket 6.0 release. Making a simple websocket server will now throw a AttributeError on connection of a websocket client.

Here's a simple websocket sanic app along with the stack trace.

from sanic import Sanic

app = Sanic()

@app.websocket('/feed')
async def feed(request, ws):
    while True:
        data = 'hello!'
        print('Sending: ' + data)
        await ws.send(data)
        data = await ws.recv()
        print('Received: ' + data)

if __name__ == "__main__":
    app.run(host="0.0.0.0", port=8000)

Here's the stack trace produced:

[2018-07-18 11:39:04 +0100] [63160] [INFO] Goin' Fast @ http://0.0.0.0:8000
[2018-07-18 11:39:04 +0100] [63160] [INFO] Starting worker [63160]
[2018-07-18 11:39:06 +0100] [63160] [ERROR] Traceback (most recent call last):
  File "venv/lib/python3.7/site-packages/sanic/app.py", line 556, in handle_request
    response = await response
  File "venv/lib/python3.7/site-packages/sanic/app.py", line 259, in websocket_handler
    ws = await protocol.websocket_handshake(request, subprotocols)
  File "venv/lib/python3.7/site-packages/sanic/websocket.py", line 63, in websocket_handshake
    key = handshake.check_request(get_header)
  File "venv/lib/python3.7/site-packages/websockets/handshake.py", line 83, in check_request
    connection = parse_connection(headers.get('Connection', ''))
AttributeError: 'function' object has no attribute 'get'

[2018-07-18 11:39:06 +0100] - (sanic.access)[INFO][1:2]: GET ws://0.0.0.0:8000/feed  500 144
@ahopkins
Copy link
Member

I can confirm this as well. I have an instance that is running websockets==5.0.1 that works fine. When websockets is upgraded to 6.0, I get the same exception.

@jamesstidard
Copy link
Contributor Author

Looks like this has already been picked up by someone on master, though on the latest 7.0 release those checks are missing.

So this might be worth cherry picking those requirements into a point release to prevent those newly pip installing from getting a version of sanic with broken websockets, until sanic is updated to play nice with websockets>=6.0.

elhb added a commit to elhb/st_spot_detector that referenced this issue Jul 20, 2018
@jamesstidard
Copy link
Contributor Author

Equally, ran across some more things that may help others. pytest-sanic also has a related issue around Sanic's websockets. Which requires to have websockets<5 instead of websockets<=6.

websockets 4.x also uses async in its code which also makes it incompatible with Python 3.7's upgrade of async to a keyword. So currently I'm running this setup:

# Pipfile
[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
sanic = "*"
websockets = "<5.0"  # Sanic breaks on 6.0, pytest-sanic breaks on 5.0

[dev-packages]
pytest-sanic = "*"

[requires]
python_version = "3.6"  # websockets 4.0 uses async keyword from python 3.7

@james9837
Copy link

I was able to get sanic master + websockets 6 to allow websocket connections with some small tweaks to sanic/websockets.py.

@baeriswyld
Copy link

the solution of james9837 worked for me!

Lagicrus added a commit to Portsmouth-Computing/py_chator that referenced this issue Aug 3, 2018
Attempt to fix issue related to sanic-org/sanic#1264
@baeriswyld
Copy link

will this fixed be merge to the version ?

@rubik
Copy link

rubik commented Aug 15, 2018

@ashleysommer Could you make a minor release with these changes? Websocket support is completely broken otherwise. Thanks!

@ahopkins
Copy link
Member

ahopkins commented Aug 15, 2018

I spent my weekend rebuilding my websocket gateway to run in websockets alone, and scraping sanic. It was not fun, but necessary because of this.

Hopefully the sanic team will get around to the huge backlog and make a release soon.

@ashleysommer
Copy link
Member

@rubik I'm not a sanic project owner or collaborator. I can't do releases. People have been wanting a new release to for months. Nothing I can do about it.

@garyo
Copy link
Contributor

garyo commented Aug 16, 2018

@ahopkins, @rubik: I've been using the github version directly in my project which works fine.
https://pip.pypa.io/en/stable/reference/pip_install/#git
I know, not ideal, but in absence of new releases...

@ahopkins
Copy link
Member

@garyo I appreciate that... but for a variety of reasons it's not an option for us.

@rubik
Copy link

rubik commented Aug 16, 2018

@ashleysommer Sorry, I was mistaken. Can we ping @r0fls then?

@ashleysommer
Copy link
Member

ashleysommer commented Aug 17, 2018

@ahopkins Im replying to your comment in this thread: #1245 (comment)
@garyo you may be interested in this too.
I have the same issue at my workplace. I use sanic in some projects, but its my own custom version, based on Sanic master with a bunch of patches applied.

Inspired by this, a few weeks ago I created a new project.
https://github.com/ashleysommer/pynecktie

PyNecktie is serious Sanic for serious business. It literally just relies on Sanic master, and inherits and extends every class with new names to make it look like its not Sanic. It has its own small test suite to ensure it is working correctly. Its aimed at devs who want to pitch this tool to their boss for use in an upcoming project, while hopefully avoiding the stigma of Sanic. It advertises it self as a "High Performance Python 3 Microservices Framework" rather than a HTTP server in an attempt to appeal to a non-tech-savvy audience.
The project is still in its infancy, I did have it pinned to [email protected] but but yesterday I rebased it to sanic@master, and fixed a bunch of issues.
I intend to have a professional-looking website/homepage for the project, hosted docs, and do releases regularly, whenever new features or fixes are added to Sanic.

@ahopkins
Copy link
Member

ahopkins commented Aug 17, 2018

@ashleysommer looks awesome... Checking it out now.

@alfonsodg
Copy link

Right now i have to fork sanic and include the patch from @james9837 to work properly, when you estimate that the 0.8 release will include that patch?

@DrPyser
Copy link

DrPyser commented Sep 19, 2018

I'm using python 3.7 for my project, and I want to use both sanic and websockets(for the websocket client). Problem is, pre-6.0 websockets client breaks in 3.7, so I need the 6.0 version. So I guess I can't use sanic(or use something else for the websocket client) until the next release. So looking forward to it.

Anyway, thanks for the great work!

@sjsadowski
Copy link
Contributor

This is addressed in PR#1304

@sjsadowski sjsadowski added this to the 18.12 LTS milestone Sep 25, 2018
@seemethere
Copy link
Member

Closing this with the merge of #1304

@tadejmagajna
Copy link

I can confirm problem persists with Python 3.7 and 3.6. I temporarily fixed the issue by downgrading websockets to websockets==6.0

@sjsadowski
Copy link
Contributor

The problem should not exist in 0.8.3 or 18.12LTS, can you provide example code you're using, along with platform so that I can test and verify?

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

No branches or pull requests