Skip to content

socket-io: Closing Browser Tab and OnDisconnect Event #49

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

Open
nmilallos97 opened this issue Jan 25, 2023 · 7 comments
Open

socket-io: Closing Browser Tab and OnDisconnect Event #49

nmilallos97 opened this issue Jan 25, 2023 · 7 comments
Assignees
Labels
bug Something isn't working upcoming fix This is in the queue to be fixed soon. There should be directions in the bug when/what the fix is.

Comments

@nmilallos97
Copy link

Upon exploring the library, I noticed that the OnDisconnect event does not trigger when closing the browser tab of an active socket connection.

However calling socket.disconnect() on client side triggers the OnDisconnect event.

@njones
Copy link
Owner

njones commented Feb 10, 2023

This library is a server implementation of the SocketIO library. It's not a client which is what will run the the browser. So closing a browser shouldn't trigger any events in this library.

Or I may be missing something. Can you point out where in the js version of the SocketIO library there is a OnDisconnect event triggered when the browser is closed, so that I can see what's done to emulate it?

@nmilallos97
Copy link
Author

nmilallos97 commented Feb 20, 2023

I see, because I used a different go library before that has this feature, namely: https://github.com/googollee/go-socket.io. Sadly this library uses an outdated version of socket.io server which won't work with our front-end technologies.

Here's the documentation with regards to the disconnect trigger:
https://socket.io/docs/v4/how-it-works/#disconnection-detection

  • socket.disconnect() is called on the server-side or on the client-side

For now I just forked the library and modified some stuff to have this mechanism work. In my case, I just modified that if the connection timed out it will trigger the onDisconnect event provided the said connection hasn't been disconnected manually by the client.

@njones
Copy link
Owner

njones commented Feb 20, 2023

Ah, I see. I missed that functionality in the documentation!

I saw your fork... I think there could be a race condition around the setting of the disconnect boolean and calling disconnect(), but I'm not positive. (in transport/transport.go:80 and transport/transport.go:84 in commit fdc1a5a)

Would you consider adding a test around your change and submitting a Pull Request? This would be a good addition to the library. If you don't feel comfortable or have the time, please let me know, and I'll add some equivalent code.

@njones njones self-assigned this Mar 8, 2023
@njones njones added the bug Something isn't working label Mar 8, 2023
@njones
Copy link
Owner

njones commented Mar 8, 2023

I will add this functionality in, over the next week. Thanks for reporting it!

@StephenSorriaux
Copy link

Hello,

Thank you for the project.

Just wanted to add that this would be a great thing to have. I did something on my side based on what @nmilallos97 already did in order to trigger the OnDisconnected when the websocket is closed (ie. when a client leaves, see StephenSorriaux@a44d608). It is probably not the best way to do it (only added for websocket transport, and for the v4 server for instance) but just wanted to share in case you have time for some comments.

@laagland
Copy link

I will add this functionality in, over the next week. Thanks for reporting it!

Hi @njones, is this something that you're still planning to include? Would really like to have this functionality.

@njones njones added the upcoming fix This is in the queue to be fixed soon. There should be directions in the bug when/what the fix is. label Jan 23, 2024
@njones
Copy link
Owner

njones commented Jan 23, 2024

Propagate the Disconnect() event through the session within the transport.go function.

Projected completion: Jan. 29, 2024

@njones njones changed the title Closing Browser Tab and OnDisconnect Event socket-io: Closing Browser Tab and OnDisconnect Event Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upcoming fix This is in the queue to be fixed soon. There should be directions in the bug when/what the fix is.
Projects
None yet
Development

No branches or pull requests

4 participants