heroku-python / flask-sockets

[DEPRECATED] Alternative: https://github.com/miguelgrinberg/flask-sock
MIT License
1.74k stars 167 forks source link

Incompatible with Werkzueg>=2.0 #81

Open rmorshea opened 3 years ago

rmorshea commented 3 years ago

I'm experiencing werkzeug.routing.WebsocketMismatch: 400 Bad Request after the recent release of werkzueg and flask. I'm not really sure why this is, but the docs state:

you receive a WebsocketMismatch exception if the only match is a WebSocket rule but the bind is an HTTP request, or if the match is an HTTP rule but the bind is a WebSocket request.

Here's the traceback I'm observing:

Traceback (most recent call last):
  File "/venv/lib/python3.9/site-packages/gevent/pywsgi.py", line 999, in handle_one_response
    self.run_application()
  File "/venv/lib/python3.9/site-packages/geventwebsocket/handler.py", line 75, in run_application
    self.run_websocket()
  File "/venv/lib/python3.9/site-packages/geventwebsocket/handler.py", line 52, in run_websocket
    list(self.application(self.environ, lambda s, h, e=None: []))
  File "/venv/lib/python3.9/site-packages/flask/app.py", line 2464, in __call__
    return self.wsgi_app(environ, start_response)
  File "/venv/lib/python3.9/site-packages/flask_sockets.py", line 40, in __call__
    handler, values = adapter.match()
  File "/venv/lib/python3.9/site-packages/werkzeug/routing.py", line 2026, in match
    raise WebsocketMismatch()
rmorshea commented 3 years ago

I think this may be a knock on effect of https://github.com/heroku-python/flask-sockets/issues/80

rmorshea commented 3 years ago

Never mind. I think this is distinct.

sphaero commented 3 years ago

Had the same issue. Fixed it by forcing the flask version to 1.1.4. There was a Flask major release (2.0.0) just a few days ago.

rmorshea commented 3 years ago

Yup that was my solution too. You should be able to do the more generic flask<2.0 though.

dbermuehler commented 3 years ago

@tomviner provided a fix for this issue with his pull request #82. Will this one be merged soon to master to fix the issue or are there any obstacles?

atzannes commented 3 years ago

seems like it would help a lot of people to have a release of flask-sockets where its Flask dependency is constrained to flask<2.0

rmorshea commented 3 years ago

I how that would play with pip's new resolver. Would it backtrack to a version that supported flask>=2.0, thus negating the benefits of the constrained release of flask-sockets?

atzannes commented 3 years ago

Never mind. I think this is distinct.

By the way, it is distinct but technically flask-sockets depends on flask which in turn depends on werkzeug (in other words flask-sockets does not depend directly on werkzeug), which is why the title and the body of this issue are a bit incongruous.

I how that would play with pip's new resolver. Would it backtrack to a version that supported flask>=2.0, thus negating the benefits of the constrained release of flask-sockets?

Very good question. I thought a lot about how to answer. Here's the short version. I could be wrong but I'm pretty sure it will work properly because the depends-on constraints define a directed acyclic graph with the root of the graph being the package that's the argument of pip install. This means that there is a complexity advantage for the constraint resolution problem of finding compatible versions for the dependencies to start with packages that are closer to the root of the graph, and because flask-sockets depends on flask it will always be closer to the root than flask for a reasonable definition of distance.

A perhaps more intuitive way to reach the same conclusion is that flask-sockets contains information on what it wants out of flask (whereas flask places no constraints on flask-sockets), and pip must download a version of a package to figure out what its dependencies and dependency constraints are, which is expensive in time, space, and bandwidth. So, it is much more advantageous for pip to "backtrack" on the flask version than on the flask-sockets version.

An example of the same problem and its solution can be seen in flask itself. Flask depends on werkzeug and when werkzeug==2.0.0 came out, it had backwards incompatible changes. At the time Flask=1.1.2 was current and required werkzeug>0.15 (here), so they released Flask==2.0.0 which requires werkzeug>=2.0 (here) and Flask==1.1.3 which requires werkzeug >0.15, <2.0 (here).

I think doing something similar here is probably the right thing, especially as it sounds there is a concern PR #82 might result in incoherent behavior when paired with werkzeug < 2 (i.e., Flask < 2.0). Specifically, I think the smoothest way forward is:

tomviner commented 3 years ago

Thanks @atzannes this sounds well thought out to me.

atzannes commented 3 years ago

Let me know if I can help push the ball forward or if there's a rough expectation for when this will be done because right now flask-sockets is blocking me from upgrading Flask, Werkzeug, and (indirectly) Click.

If this is going to take much longer, I'll just fork flask-sockets and use that, no worries.

edmorley commented 3 years ago

@atzannes Hi! Sorry for the delayed reply. This repository isn't actively monitored. I unfortunately don't have the time to maintain this (inherited) project, and transferring a repo/package has it's own set of risks. As such I would recommend forking - and I'm happy to link out to the fork from this repo before archiving it. I would recommend creating as a new repo (and not via the GitHub fork feature), so that it appears as a top level project and not a GH fork of this one (which breaks things like code search, even ignoring discoverability).

Many thanks :-)

ibrewster commented 3 years ago

I have forked this repository, as suggested, and after applying the patch got it working for me, but I'm a bit concerned about this project no longer being maintained.

Is there a different project that can be recommended to provide WebSocket support that is currently maintained? All google gives me is flask-scoketio, which does not provide WebSocket support at all (as far as I can tell).

edmorley commented 3 years ago

Someone needs to create a fork and maintain it, then everyone can use that. This repository is going to be archived soon, since I don't have the bandwidth to maintain it (I don't use Flask regularly for a start).

wasauce commented 2 years ago

@atzannes @ibrewster - Are either of you planning to setup a top level project?

ibrewster commented 2 years ago

TBH, I was planning to simply use this (with the patch applied) as-is until it stopped working due to some future upgrade, and then go looking for something else. Although libraries that provide web socket support for Flask do seem to be few and far between :-)

atzannes commented 2 years ago

@wasauce I thought about setting up a top level project, but I'm really not using Flask in my day to day and I went through the issues on this repo and I don't think I would have the bandwidth to get familiar enough with Flask to be able to address those issues. So, at least for now, I have also forked this repo and applied the patch (https://github.com/APrioriInvestments/flask-sockets)

wasauce commented 2 years ago

@atzannes @ibrewster thanks very much. If I end up going this route and using flask-sockets -- I'll follow-up and make a top level project. Still investigating and tinkering over here.

rmorshea commented 2 years ago

@wasauce, if you find an alternative to this project for implementing web sockets in Flask it'd be great if you could post back here with your findings. I was unable to discover one myself when I was first looking.

avble commented 2 years ago

@rmorshea

I have encountered the same issue, werkzeug.routing.WebsocketMismatch: 400 Bad Request.

And the fix is as below commit. The fix has been confirmed at my end. Hope it help you

https://github.com/avble/flask-sockets/commit/dd783513af0b1a0120112bdcd7de1b43342a646b

edmorley commented 2 years ago

For anyone looking for a replacement to this (deprecated) project, I would check out: https://github.com/miguelgrinberg/flask-sock