fix the order of triggered disconnect event

[sillygod]

I think the order for triggering disconnect is wrong.

[miguelgrinberg]

Is there a specific problem you are trying to address with this change?

The problem is that when the client disconnects there is no way to send events or actually do anything with that client connection, so it makes sense to remove it before calling the disconnect handler, since that client is unreachable anyway.

[sillygod]

well, I want the disconnected client to broadcast the news that it is disconnected and let certain client know that.

self.manager.disconnect(sid, n)
self._trigger_event('disconnect', n, sid)

so in original order, I actually can not do anything related to socket in disconnect function.

In your example

@sio.on('disconnect', namespace='/test')
def test_disconnect(sid):
    print('Client disconnected')

Actually, you just did simple print but how do I do something with sid?

and I am curious about this part of code

if namespace == '/' and self.manager.is_connected(sid, namespace):
    self._trigger_event('disconnect', '/', sid)
    self.manager.disconnect(sid, '/')

why it call self._trigger_event before self.manager.disconnect here ?

sorry my English.. I've already try hard to make my description clear.

[miguelgrinberg]

Your English is fine. :)

I think the way disconnects are handled are very consistent. Basically what I tried to do is prevent is that you send an event from the disconnect handler to the user that got disconnected, as that will obviously fail. There are several ways to trigger a disconnection, and as I said above, they aren't handled in a consistent way. Examples:

• If the server initiates a disconnection, then it is okay to send an event to the disconnected client. Because of the order of the two functions, when the disconnect handler is called, the client is still connected.
• If the client initiates a disconnection by closing the browser or tab, then when the disconnect handler is called the client is already gone, so any attempts to send that client something will fail. I prevent this by removing the client before the disconnect handler is invoked.
• When the client does a clean disconnect by calling the disconnect function (instead of leaving the page), the current code is inconsistent, as it calls the disconnect handler before removing the client. If anything, I would think this case needs to be changed to work like the previous one.

so in original order, I actually can not do anything related to socket in disconnect function.

This part I don't understand. What do you want to do? You do get the sid of the client. If you broadcast a message, all other clients will receive it, but not the disconnected one, since that one is already gone. Why is that a problem?

[sillygod]

the following code is part ot my project.

def on_disconnect(self, sid):
        user = self._get_request(sid).user
        self.broadcast_to_rooms(sid, 'whoisoffline', {'user': user.token, 'msg': 'success'})

        if user.token in ONLINE_USERS:
            ONLINE_USERS.pop(user.token)

        return True

here the thing I want to do is the client to notify certain socket that it is disconnected ( in some situation like, user close the browser window or network connection lost.

actually, it will fail if the user closed browser.

I think the reason is

self.manager.disconnect(sid, n)
self._trigger_event('disconnect', n, sid)

it will make sid be removed before it call the function on_disconnect. That's why I want the order to be

self._trigger_event('disconnect', n, sid)
self.manager.disconnect(sid, n)

[miguelgrinberg]

Can you show me the code for your broadcast_to_rooms method?

[sillygod]

def broadcast_to_rooms(self, sid, event, data, namespace, skip_sid=None, callback=None):
        """broadcast message to the rooms you are joined
        """
        room_tokens = self.get_rooms(sid, namespace)

        if namespace not in self.rooms:
            return
        for room in room_tokens:
            if room not in self.rooms[namespace]:
                continue
            for sid in self.get_participants(namespace, room):
                if sid != skip_sid:
                    if callback is not None:
                        id = self._generate_ack_id(sid, namespace, callback)
                    else:
                        id = None

                    self.server._emit_internal(sid, event, data, namespace, id)

I've tried it will work if the client invoke the following method

def on_disconnect_request(self, sid):
        print('enter disconnect request')
        namespace.server.disconnect(sid, namespace=self.name)

[miguelgrinberg]

Okay, I do understand what you are trying to do now. You do not want to send anything to the disconnected client, but you want to know in which rooms that client was, so that you can broadcast something to the remaining clients in those rooms. Makes sense.

I'm going to allow this change. I'll test it a little bit to make sure nothing is affected and if everything is all right I'll merge it. Thanks!

[sillygod]

Thanks :)

By the way, it is so hard to communicate with others by second language.

I will still improve my english in order to make my description more clear and understand what other people want to express.

[miguelgrinberg]

it is so hard to communicate with others by second language

Yes, it's not easy, but you are doing a pretty good job. English is also a second language for me, by the way, but I have been speaking it for a long time.