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

Allow websocket connections (e.g. python-socketio) on child app #10

Closed
amaurymercier opened this issue Jun 19, 2019 · 8 comments
Closed

Comments

@amaurymercier
Copy link

amaurymercier commented Jun 19, 2019

Hi (it's me again... Thanks a lot for the last issue by the way !)

It seems not possible to create a websocket server, e.G. using python-socketio, on a child / dispatched app.
Is it expected behaviour ? Or am I doing it the wrong way ?

The error code happens in python-engineio but I think it comes from the dispatcher (it works fine when the socketio server is attached to the main app)

Here's the code to reproduce :

Requirements:

Sanic-Dispatcher==0.7.0.0
sanic==19.3.1
python-socketio==4.1.0

Server.py

from sanic import Sanic
from sanic.response import text
from sanic_dispatcher import SanicDispatcherMiddlewareController
import socketio

main_app = Sanic(__name__)
dispatcher = SanicDispatcherMiddlewareController(main_app)


@main_app.get('/')
def hello_main(request):
    """Route main."""
    return text('hello, main world !')


child_app = Sanic(__name__)
sio = socketio.AsyncServer(async_mode='sanic')

sio.attach(child_app)


@child_app.route('/')
async def index(request):
    return text('Hi.')


@sio.event
async def disconnect_request(sid):
    await sio.disconnect(sid)


@sio.event
async def connect(sid, environ):
    await sio.emit('my_response', {'data': 'Connected', 'count': 0}, room=sid)


@sio.event
def disconnect(sid):
    print('Client disconnected')


dispatcher.register_sanic_application(
    child_app,
    '/child'
)

if __name__ == '__main__':
    main_app.run(port=8001, debug=True)

client.py :

import socketio

sio = socketio.Client()
url = 'ws://localhost:8001/'
path = '/child/socket.io/'


@sio.event
def connect():
    print("Connected")


@sio.event
def disconnect():
    print("Disconnected")


sio.connect(url, socketio_path=path)

Error code when running server.py then connect.py :

[2019-06-19 10:55:55 +0200] [16801] [INFO] Goin' Fast @ http://127.0.0.1:8001
before_start <sanic.app.Sanic object at 0x7fcdfb998e80>
[2019-06-19 10:55:55 +0200] [16808] [INFO] Starting worker [16808]
[2019-06-19 10:55:58 +0200] - (sanic.access)[INFO][127.0.0.1:47244]: GET http://localhost:8001/socket.io/?transport=polling&EIO=3&t=1560934558.9213946  200 166
Fatal error on transport TCPTransport
protocol: <sanic.server.HttpProtocol object at 0x7fcde9927a48>
transport: <TCPTransport closed=False reading=False 0x2a50478>
Traceback (most recent call last):
  File "uvloop/handles/stream.pyx", line 827, in uvloop.loop.__uv_stream_on_read_impl
  File "/home/.../.virtualenvs/.../lib/python3.6/site-packages/sanic/server.py", line 267, in data_received
    self.parser.feed_data(data)
  File "httptools/parser/parser.pyx", line 196, in httptools.parser.parser.HttpParser.feed_data
httptools.parser.errors.HttpParserUpgrade: 328
[2019-06-19 10:55:58 +0200] [16808] [ERROR] Exception occurred while handling uri: 'ws://localhost:8001/socket.io/?transport=websocket&EIO=3&sid=4ca8ac6c0cdc43a09c9426829ae9f5be&t=1560934558.930295'
Traceback (most recent call last):
  File "/home/.../.virtualenvs/.../lib/python3.6/site-packages/sanic/app.py", line 917, in handle_request
    response = await response
  File "/usr/lib/python3.6/asyncio/coroutines.py", line 110, in __next__
    return self.gen.send(None)
  File "/home/.../.virtualenvs/.../lib/python3.6/site-packages/engineio/asyncio_server.py", line 220, in handle_request
    packets = await socket.handle_get_request(environ)
  File "/usr/lib/python3.6/asyncio/coroutines.py", line 110, in __next__
    return self.gen.send(None)
  File "/home/.../.virtualenvs/.../lib/python3.6/site-packages/engineio/asyncio_socket.py", line 89, in handle_get_request
    return await getattr(self, '_upgrade_' + transport)(environ)
  File "/usr/lib/python3.6/asyncio/coroutines.py", line 110, in __next__
    return self.gen.send(None)
  File "/home/.../.virtualenvs/.../lib/python3.6/site-packages/engineio/asyncio_socket.py", line 128, in _upgrade_websocket
    return await ws(environ)
  File "/usr/lib/python3.6/asyncio/coroutines.py", line 110, in __next__
    return self.gen.send(None)
  File "/home/.../.virtualenvs/.../lib/python3.6/site-packages/engineio/async_drivers/sanic.py", line 119, in __call__
    self._sock = await protocol.websocket_handshake(request)
AttributeError: 'HttpProtocol' object has no attribute 'websocket_handshake'
[2019-06-19 10:55:58 +0200] - (sanic.access)[INFO][127.0.0.1:47246]: GET ws://localhost:8001/socket.io/?transport=websocket&EIO=3&sid=4ca8ac6c0cdc43a09c9426829ae9f5be&t=1560934558.930295  500 144

Thanks a lot !

@ashleysommer
Copy link
Owner

Hi @amaurymercier
Thanks for opening this issue.

Yes, unfortunately this problem is caused by Sanic-Dispatcher.

The reason this happens is because the child sanic app doesn't actually operate its own http webserver, and it doesn't have a working HttpProtocol object attached. All of that is meant to be handled by the parent app, and requests/responses are just passed to and from the child app.

This means there are some things that the socketio library expects the child app can do (like receiving a websocket request), that it actually cannot, that should actually go to the parent app.

I think it might be possible to get this working with a new feature in Sanic-Dispatcher, but maybe not. I will look into it.

@ashleysommer
Copy link
Owner

ashleysommer commented Jun 20, 2019

Hi @amaurymercier
Another update. I have a fix for you.
Please install the new version of Sanic-Dispatcher v0.7.1.0 from PyPI, then modify your code to add main_app.enable_websocket()
Like so:

from sanic import Sanic
from sanic.response import text
from sanic_dispatcher import SanicDispatcherMiddlewareController
import socketio

main_app = Sanic(__name__)
main_app.enable_websocket()  # <-- This line
dispatcher = SanicDispatcherMiddlewareController(main_app)


@main_app.get('/')
def hello_main(request):
    """Route main."""
....

@amaurymercier
Copy link
Author

amaurymercier commented Jun 21, 2019

Hi,

Thanks a lot for this !!

I tested it and it works.
Strangely enough I got a problem in my app where I use CORS with your sanic-cors on the child app and the websocket app is built using blueprints. The problem happens in sanic_cors.set_cors_headers because resp was equal to []. I changed the code in this extension to test for resp is None or resp is [] instead of resp is None and it works. I'm not sure where it comes from though.
This is the same piece of code as here : ashleysommer/sanic-cors#2

Best

@ashleysommer
Copy link
Owner

ashleysommer commented Jun 23, 2019

Hi @amaurymercier
I've managed to track down the source of the bug you're seeing where resp is [].
You'll notice it never occurs the first time the client connects to the server, only if the client disconnects then reconnects within 30 seconds, before the first websocket it timed out.
When the 2nd client connection comes in, it kills the first one. That causes the first long-running HTTP upgrade connection to die, and when it does, Engineio returns a [] as the response to Sanic.

See that here:
https://github.com/miguelgrinberg/python-engineio/blob/a4404989a1c48b3522c09a7f6335f2ad401805a2/engineio/asyncio_server.py#L256

The built-in sanic response handler sees the HTTP Response is not None so it tries to apply response-middleware to it. Being that the response from the closed Engineio connection is [] this tends to break common response-middlewares, including Sanic-CORS.

I can work around it, as you've suggested, by including an extra check for resp == [] in Sanic-CORS (which I will do), but I wonder if this is something that should be fixed in either Python-Engineio or Sanic.

EDIT: To add, just clarifying, this latest error is not an issue in Sanic-Dispatcher, nor in Sanic-Plugins-Framework, and it is not a result of running on a child app, and is not related to using Blueprints.

@ashleysommer
Copy link
Owner

ashleysommer commented Jun 24, 2019

@amaurymercier
I've just released a new version of Sanic-CORS, v0.9.8.post1 with a fix (workaround) for the above mentioned bug. Can you please test it to check if it makes the error go away in your application?

@ashleysommer
Copy link
Owner

Another comment to add, looks like it is a bug in python-engineio miguelgrinberg/python-engineio#120

@amaurymercier
Copy link
Author

I see, interesting.

The v0.9.8.post1 does make the error go away in my app as expected
Thanks again !

@ashleysommer
Copy link
Owner

Closing this issue now. If you come across any more problems, make a new issue. Thanks.

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

No branches or pull requests

2 participants