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

Automatically reconnect on offline event #9299

Conversation

msujew
Copy link
Member

@msujew msujew commented Apr 6, 2021

What it does

Fixes #9298.

As the bug report over at chromium indicates, listening to the offline event of the browser enables us to manually tell the socket to reconnect. This event is usually fired during a change of network.

I tested this behavior on an Ubuntu 20.04 machine. Running a VM on Windows 10 didn't allow me to reproduce the network issue. I also looked for any regression issues under Firefox and Windows, but everything seems to work normally even with the fix in place.

How to test

  1. Start Theia in a chromium based browser on a Linux distribution.
  2. Change to another Wifi network.
  3. The websocket connection will be re-established, indicated by the banner changing from online to offline and finally back to online.

Review checklist

Reminder for reviewers

@vince-fugnitto vince-fugnitto added messaging issues related to messaging websocket issues related to websockets/messaging labels Apr 6, 2021
@msujew msujew force-pushed the websocket-reconnection-network-change branch from b3b8ac9 to b6e0709 Compare April 6, 2021 20:34
Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM. Although I wasn't able to reproduce the issue on master using Linux and Chrome. It is working for me with and without this change.

@vince-fugnitto
Copy link
Member

Code LGTM. Although I wasn't able to reproduce the issue on master using Linux and Chrome. It is working for me with and without this change.

I was unable to reproduce the issue as well on both master and this pull-request.

@paul-marechal
Copy link
Member

paul-marechal commented Apr 12, 2021

@Darkgaja can you confirm that you have the issue as of master and that this PR fixes it for you? I don't see any regression, so if it fixes it for you then I'll merge.

@msujew
Copy link
Member Author

msujew commented Apr 13, 2021

@marechal-p @vince-fugnitto Just tested it on the master and the issue still persists:

2021-04-13_08-46-40.mp4

(Saving doesn't work, git synchronisation as well. Waiting doesn't help)

My changes seem to fix these issues:

2021-04-13_11-15-32.mp4

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Darkgaja thank you for providing the screenshares, they helped me reproduce the issue on master and I confirmed that the pull-request addresses the issue nicely 👍

@paul-marechal paul-marechal merged commit 148e70c into eclipse-theia:master Apr 13, 2021
@msujew msujew deleted the websocket-reconnection-network-change branch April 13, 2021 14:08
@vince-fugnitto vince-fugnitto added this to the 1.13.0 milestone Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
messaging issues related to messaging websocket issues related to websockets/messaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Websocket does not reconnect when changing WIFI networks on Linux distributions
3 participants