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

disconnect() function does not allow arguments #1017

Closed
lgprobert opened this issue Jul 2, 2019 · 18 comments
Closed

disconnect() function does not allow arguments #1017

lgprobert opened this issue Jul 2, 2019 · 18 comments
Assignees
Labels

Comments

@lgprobert
Copy link

lgprobert commented Jul 2, 2019

Using python-socketio client connects to flask-socketio server with namespace. Two way connection works fine and sio client even can receive server emitting 'disconnect' events.

But when I want to disconnect client at flask-server, disconnect() function without argument seems not working, server side on_disconnect handler can see the disconnect event but client does not receive disconnect event.

Then I tried to explicitly specify sid and namespace arguments in this way:

disconnect(request.sid, namespace=request.namespace)

But flask-socketio server throws following error message when running this line:

    disconnect(sid=request.sid, namespace=request.namespace)
TypeError: disconnect() got an unexpected keyword argument 'sid'

What I can achieve the same effect by now is using following a few more lines:

socketio.emit('aroom', 'disconnect', namespace='/test', room=request.sid)
return

This way effectively just notify client to disconnect from client side.
What I am looking for is something like sio.disconnect() that can happen at server side.

Thanks

@miguelgrinberg
Copy link
Owner

The disconnect in the client does not take any arguments, it disconnects all namespaces from a single call. What is the problem that you are having with that? Sorry, but I don't understand what you are trying to do.

@lgprobert
Copy link
Author

Hi Miguel,

I try to disconnect client at server side like I did at client side with sio.disconnect(). Unfortunately the flask-socketio function socketio.disconnect() at server side does not have client get notified when there is no arguments are set, and run into problem when I gave namespace and sid arguments to the function.

So the problem is about how flask-socketio's function disconnect() can terminate a living connection at server side.

Hope this can help you understand more clearly.

Thanks

@miguelgrinberg
Copy link
Owner

@lgprobert The example application in this repository has a server-side disconnect function. I don't see any problems with it. You may need to create a small example application that shows the problem you are having to help me reproduce the problem here.

@lgprobert
Copy link
Author

OK, here are my single tests:

server side: app.py:

from flask import Flask, request, current_app
from flask_socketio import SocketIO, emit, disconnect
import eventlet
from threading import Lock
import random
eventlet.monkey_patch()

app = Flask(__name__)
app.config['SECRET_KEY'] = 'secret!'
async_mode = 'eventlet'
socketio = SocketIO(async_mode='eventlet')
socketio.init_app(app)
thread = None
thread_lock = Lock()

@app.route('/')
def index():
    return "test socketio\n"

def background_task(app, sid):
    app.app_context().push()
    print('{} is inside background thread'.format(sid))
    for i in range(3):
        anum = str(random.randint(100001, 999999))
        emit('my_response', anum, namespace='/test', room=sid)
    return

@socketio.on('longtime', '/test')
def bgtask(message):
    global thread
    print('sid type: ', type(request.sid))
    emit('my_response', 'background task', namespace='/test')
    with thread_lock:
        if thread is None:
            thread = socketio.start_background_task(background_task, current_app._get_current_object(), sid=request.sid)
    print('disconnect now')
    #emit('my_response', 'disconnect')
    disconnect(sid=request.sid, namespace='/test')

@socketio.on('trouble', '/test')
def trouble(message):
    print('event handler to show trouble of server side disconnect')
    emit('my_response', 'trouble begins with disconnect')
    #disconnect()
    disconnect(sid=request.sid, namespace='/test')

@socketio.on('good', namespace='/test')
def good(message):
    print('everything will be good')
    emit('my_response', 'received hello', namespace='/test')
    #emit('my_response', 'disconnect')
    emit('my_response', 'disconnect', namespace='/test')

if __name__ == '__main__':
    socketio.run(app, host='localhost', port=5000, debug=True, log_output=True)

Here is the client codes: client4.py

import socketio
import sys

sio = socketio.Client()
if sys.argv[1] == 'good':
    action = 'good'
elif sys.argv[1] == 'trouble':
    action = 'trouble'
elif sys.argv[1] == 'long':
    action = 'longtime'
else:
    print('wrong input, either good or trouble')
    sys.exit(1)

@sio.on('my_response', namespace='/test')
def on_response(data):
    if data != 'disconnect':
        print('received data: ', data)
    else:
        sio.disconnect()

@sio.event(namespace='/test')
def connect():
    print('connected to server')
    sio.emit(action, 'hello', namespace='/test')

@sio.event(namespace='/test')
def disconnect():
    print("I'm disconnected!")

sio.connect('http://localhost:5000', namespaces=['/test'])
print('my connected sid is: ', sio.sid)

With this simple app, I found flask-socketio disconnect() can be configured with arguments (but I did have that trouble when putting it at other places, maybe decorator?).

But now the problem is if I run client4.py with either CLI argument long or trouble that would hit the disconnect() function, the server only sends a disconnect event to client. Client receives the event but the websocket connection is intact. Again, the only way to disconnect client connection from server side are:

  • emit a disconnect event to client
  • sio at python-socket then execute sio.disconnect() from the response handler like on_response() at my sample above.

@miguelgrinberg
Copy link
Owner

Thanks, now I understand.

What I think is missing here is that when the server issues the disconnect the WebSocket connection is not aborted. I'll look into it.

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Aug 3, 2019

@lgprobert I have attempted a fix to this problem. The fix is in package python-socketio. Can I ask you to install the master branch of that package from git and verify if the problem is resolved?

The change that I've made is that when the server disconnects a client, the WebSocket connection is also closed, this wasn't being done. Thanks!

The commit with the fix is miguelgrinberg/python-socketio@516a295.

@stephenrs
Copy link

stephenrs commented May 2, 2021

I found my way here because I was unable to call disconnect with a sid argument as described in the docs as follows (using flask-socketio v5.0):

flask_socketio.disconnect(sid=None, namespace=None, silent=False)
Disconnect the client.

This function terminates the connection with the client. As a result of this call the client will receive a disconnect event.

Calling disconnect as above triggers an "attribute disconnect does not exist" error (or something similar). However, I discovered that the following works:

flask_socketio.server.disconnect(sid=sid)

So...

  1. Am I misunderstanding that section of the documentation?
  2. Is calling disconnect() as in the working example above not recommended?
  3. Since this problem matches the subject line of the OP, I'm posting here, but let me know if there's a better place to pose this.

Thanks

@miguelgrinberg
Copy link
Owner

@stephenrs what you are doing is a bad idea. You are using a private attribute of the Socket.IO extension (server), which can change at any time. You should only use publicly documented functions.

The disconnect() function does accept a sid argument, so I do not understand why you get an error. Provide the exact error and stack trace, please.

@stephenrs
Copy link

stephenrs commented May 2, 2021

@miguelgrinberg I'm trying to call disconnect from a class method, outside of the event context, and here's the relevant code:

    def disconnect_all(self):
        log.error('PANIC!! DISCONNECT ALL CLIENTS!!')
        for room in self.rooms.values():
            for sid in room['sids']:
                # socketio.server.disconnect(sid=sid) # works
                socketio.disconnect(sid=sid) # doesn't work

The disconnect_all() method gets called as part of handling an exception, so the exception that triggered the call is included in the full stack trace here:

ERROR: WebsocketBroker: PANIC!! DISCONNECT ALL CLIENTS!!!!!
Traceback (most recent call last):
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/urllib3/connectionpool.py", line 699, in urlopen
    httplib_response = self._make_request(
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/urllib3/connectionpool.py", line 394, in _make_request
    conn.request(method, url, **httplib_request_kw)
  File "/usr/local/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1255, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/local/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1301, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/local/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1250, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/local/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1010, in _send_output
    self.send(msg)
  File "/usr/local/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 950, in send
    self.connect()
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/requests_unixsocket/adapters.py", line 41, in connect
    sock.connect(socket_path)
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/gevent/_socketcommon.py", line 628, in connect
    raise _SocketError(result, strerror(result))
FileNotFoundError: [Errno 2] No such file or directory

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/requests/adapters.py", line 439, in send
    resp = conn.urlopen(
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/urllib3/connectionpool.py", line 755, in urlopen
    retries = retries.increment(
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/urllib3/util/retry.py", line 532, in increment
    raise six.reraise(type(error), error, _stacktrace)
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/urllib3/packages/six.py", line 734, in reraise
    raise value.with_traceback(tb)
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/urllib3/connectionpool.py", line 699, in urlopen
    httplib_response = self._make_request(
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/urllib3/connectionpool.py", line 394, in _make_request
    conn.request(method, url, **httplib_request_kw)
  File "/usr/local/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1255, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/local/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1301, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/local/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1250, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/local/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1010, in _send_output
    self.send(msg)
  File "/usr/local/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 950, in send
    self.connect()
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/requests_unixsocket/adapters.py", line 41, in connect
    sock.connect(socket_path)
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/gevent/_socketcommon.py", line 628, in connect
    raise _SocketError(result, strerror(result))
urllib3.exceptions.ProtocolError: ('Connection aborted.', FileNotFoundError(2, 'No such file or directory'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./app_broker.py", line 78, in monitor
    resp = requests.get('http+unix://listener-feed.sock/rooms')
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/requests_unixsocket/__init__.py", line 51, in get
    return request('get', url, **kwargs)
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/requests_unixsocket/__init__.py", line 46, in request
    return session.request(method=method, url=url, **kwargs)
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/requests/sessions.py", line 542, in request
    resp = self.send(prep, **send_kwargs)
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/requests/sessions.py", line 655, in send
    r = adapter.send(request, **kwargs)
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/requests/adapters.py", line 498, in send
    raise ConnectionError(err, request=request)
requests.exceptions.ConnectionError: ('Connection aborted.', FileNotFoundError(2, 'No such file or directory'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "src/gevent/greenlet.py", line 906, in gevent._gevent_cgreenlet.Greenlet.run
  File "./app_broker.py", line 98, in monitor
    self.disconnect_all()
  File "./app_broker.py", line 191, in disconnect_all
    socketio.disconnect(sid=sid)
AttributeError: 'SocketIO' object has no attribute 'disconnect'
2021-05-02T17:55:13Z <Thread at 0x11eda9590: <bound method ListenerClient.monitor of <app_broker.ListenerClient object at 0x11ee0a100>>> failed with AttributeError

One of my goals in using an explicit disconnect() is to take advantage of the reconnect feature that is built into the Socketio javascript client, without writing explicit reconnect logic. In my setup, if a PANIC failure like this happens on one node, there's probably another one that JS clients can connect to.

Thanks for taking a look.

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented May 2, 2021

@stephenrs disconnect is a standalone function, not a method of the socketio object. Try this:

from flask_socketio import disconnect

# ...

    def disconnect_all(self):
        log.error('PANIC!! DISCONNECT ALL CLIENTS!!')
        for room in self.rooms.values():
            for sid in room['sids']:
                disconnect(sid=sid) # doesn't work

@stephenrs
Copy link

stephenrs commented May 2, 2021

@miguelgrinberg I've tried both of the blocks below, but received the same stack trace as shown.

  def disconnect_all(self):
       log.error('PANIC!! DISCONNECT ALL CLIENTS!!')
       for room in self.rooms.values():
           for sid in room['sids']:
               # socketio.server.disconnect(sid=sid)
               # with app.app_context():
               disconnect(sid=sid)
    def disconnect_all(self):
        log.error('PANIC!! DISCONNECT ALL CLIENTS!!')
        for room in self.rooms.values():
            for sid in room['sids']:
                # socketio.server.disconnect(sid=sid)
                with app.app_context():
                    disconnect(sid=sid)
Traceback (most recent call last):
  File "src/gevent/greenlet.py", line 906, in gevent._gevent_cgreenlet.Greenlet.run
  File "./app_broker.py", line 98, in monitor
    self.disconnect_all()
  File "./app_broker.py", line 192, in disconnect_all
    disconnect(sid=sid)
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/flask_socketio/__init__.py", line 988, in disconnect
    socketio = flask.current_app.extensions['socketio']
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/werkzeug/local.py", line 347, in __getattr__
    return getattr(self._get_current_object(), name)
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/werkzeug/local.py", line 306, in _get_current_object
    return self.__local()
  File "/Users/stephen/www/feed/venv3/lib/python3.9/site-packages/flask/globals.py", line 52, in _find_app
    raise RuntimeError(_app_ctx_err_msg)
RuntimeError: Working outside of application context.

However, I think there's an important piece of information that I've left out. disconnect_all() is called from within a background thread, which is started like so:

socketio.start_background_task(target=client.monitor)

I have a separate background thread in the same app that calls socketio.emit() and I haven't seen any problems (but please let me know if this is problematic). So, I assumed that socketio.disconnect() could be called from a background thread in the same way that socketio.emit() can be.

FWIW, I had this mostly working by emitting a custom event from disconnect_all(), and doing stuff on both ends of the connection to make the reconnection sequence work. But, it was messy and not bullet-proof, and didn't take advantage of the housekeeping and other magic that Socketio provides when there is a hard disconnect().

What am I misunderstanding?

Also, as far as I can tell, I'm using a consistent threading model on a single process. In case it helps, my app starts like this (and I've used queue.Queue() with success for other stuff):

from gevent import monkey, queue
monkey.patch_all()

@stephenrs
Copy link

@miguelgrinberg I guess maybe this comes down to a feature request: Would you consider exposing a disconnect() method on the socketio object so that it can be called in the same contexts that socketio.emit() can be called in? Is there a reason why this would be a bad idea?

@miguelgrinberg
Copy link
Owner

@stephenrs the problem is that your function is also called disconnect. You need to rename one of the two to a different name to remove the name collision.

Would you consider exposing a disconnect() method on the socketio object so that it can be called in the same contexts that socketio.emit()

That is not the usage I envisioned. This extension follows the Flask pattern. You aren't calling methods off of the app instance in Flask, you have standalone functions such as redirect, render_template, and so on. With Flask-SocketIO you should use the standalone emit, disconnect, etc. The socketio object is just to register your handlers.

@stephenrs
Copy link

My function is actually called "disconnect_all", so there isn't a name collision. Also, as in my initial comment, this works as desired:

socketio.server.disconnect()

So, as far as I can tell, the problem is that it's possible to emit from an external process, as described in doc section "Emitting from an External Process", but it's not possible to "Disconnect from an External Process".

In my case, the "external process" is actually just a health monitoring thread that runs a "while True:" and takes actions when certain conditions in a bigger backend process are detected. I guess maybe there are other ways that the monitoring thread can let the main process know when a PANIC condition is detected, for example, but it would just be more convenient to call disconnect() directly.

So, is there a reason that external processes should be able to emit() but not disconnect()? Am I misunderstanding something?

And, I understand that you don't recommend accessing socketio.server methods directly, for obvious reasons, but unless a better solution surfaces, freezing my versions and going with the one-liner above could be an option. It does exactly what I want it to do, as far as I can tell, and is the cleanest solution at the moment.

@miguelgrinberg
Copy link
Owner

External process have limited support, since they are not servers and consequently do not have access to the actual sockets. These days using a client in your external process might make more sense if you have advanced needs. The client running in the external process can emit to the server and ask the server to do any actions.

@stephenrs
Copy link

stephenrs commented May 4, 2021

External process have limited support, since they are not servers and consequently do not have access to the actual sockets. These days using a client in your external process might make more sense if you have advanced needs. The client running in the external process can emit to the server and ask the server to do any actions.

That's definitely an interesting idea, thanks, and I can see how it could work, so I decided to give it a quick try. The problem I'm having now though is that I'd like to avoid the TCP stack in this part of the setup. So, I'd rather use a unix domain socket to get the background thread talking to the main server, since they'll be running on the same physical server for the foreseeable future.

So, I'd like to connect to the server using something like this:

sio.connect('http+unix:///tmp/my_socket.sock')

Unfortunately, while the server is happy using UDS, it appears that the python-socketio client doesn't like it, so I'm getting "Connection refused" errors. I've also come across some semi-related chatter:

socketio/socket.io#5097)
miguelgrinberg/python-socketio#541

So, it looks like this isn't possible(?) ... unless I've configured something incorrectly.

Given the tradeoffs, I'm still leaning toward invading the privacy of socketio.server, and hoping that external process support gets more love eventually - and includes a wrapper function around socketio.server.disconnect(), or something similar. My current needs definitely go beyond the typical chat server setup, and sometimes you have to (cautiously) break the rules to do more advanced things.

I'm all ears if you have any other thoughts.

@miguelgrinberg
Copy link
Owner

The URLs with the form http+unix://... are not valid for the requests package, as far as I know, so that might not be possible, unless there is an extension or plugin that adds this in a way that is compatible with requests.

@stephenrs
Copy link

stephenrs commented May 4, 2021

@miguelgrinberg I've been using the following, and it works really well, and has helped me minimize complexity and keep everything tidy and away from TCP:

# https://pypi.org/project/requests-unixsocket/
import requests_unixsocket
requests_unixsocket.monkeypatch()

EDIT: Just for clarity, for anyone else who stumbles onto this: monkeypatching with the requests_unixsocket module has allowed me to use the standard requests module to let flask/uwsgi-based micro services talk to each other in a highly performant and convenient way, using commands of the form:

r = requests.get('http+unix://my-socket.sock/<some URI>')

So, I'll comment that in websocket-land, where performance/scale is often critical, it surprises me that it's not more common to communicate across unix sockets, which I guess I would have assumed would be the default/primary case for inter-service communication. I've personally never done anything with uwsgi that didn't use UDS...when "easier" lines up with "faster and better", I'll usually take that route :)

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

No branches or pull requests

3 participants