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

Migrate Socket.IO from Version 2 to Version 3 πŸš€ #6152

Merged
merged 8 commits into from
Feb 17, 2024

Conversation

HMarzban
Copy link
Contributor

Upgraded the Socket.IO library from version 2 to version 3.
This PR is a continuation of PR #4916.
@SamTV12345 πŸ§‘β€πŸ’»

@SamTV12345
Copy link
Member

That looks awesome. I'll review and test tomorrow. If all looks good I'll merge this. Would you be interested in also upgrading to v4 after that?

@HMarzban
Copy link
Contributor Author

Great, thanks! If it all goes smoothly, I'm up for upgrading to v4 next.

@SamTV12345
Copy link
Member

@HMarzban could you reset the test timeout in the package.json? I think that is why the load tests are failing.

@SamTV12345
Copy link
Member

When I start Etherpad I get a bad request {"code":0,"message":"Transport unknown"}

@HMarzban
Copy link
Contributor Author

HMarzban commented Feb 12, 2024

Encountered another failure. I've reviewed the etherpad-load-test, and it turns out it utilizes the etherpad-cli-client. This CLI depends on socket.io-client v2, which is the root of the problem and needs upgrading.

However, on a positive note, the backend tests passed successfully.

I also manually tested all functionalities of the client, and everything appeared to be functioning correctly.

@HMarzban
Copy link
Contributor Author

Upgrading from v3 to v4 requires more extensive refactoring, but only on the backend side.
As far as I'm aware, the client-side SDK (v3) is compatible with the v4 server-side.
Therefore, I recommend we first thoroughly test and address any potential issues with v3.

Once that's done, I can start working on upgrading the socket to v4.

@HMarzban
Copy link
Contributor Author

When I start Etherpad I get a bad request {"code":0,"message":"Transport unknown"}

In the new version of socket.io, the socket protocols such as ["xhr-polling", "jsonp-polling", "htmlfile"] are no longer available.

Therefore, you will need to update them to ["websocket", "polling"].

change the settings:

---
"socketTransportProtocols" : ["websocket", "polling"],
---

@SamTV12345
Copy link
Member

Upgrading from v3 to v4 requires more extensive refactoring, but only on the backend side. As far as I'm aware, the client-side SDK (v3) is compatible with the v4 server-side. Therefore, I recommend we first thoroughly test and address any potential issues with v3.

Once that's done, I can start working on upgrading the socket to v4.

Good idea. I'm currently fighting over on etherpad-cli-client. I guess this is because the loadtests fail. Thanks that did fix the issue. Maybe I can just update the settings when the user starts Etherpad. Otherwise we will get a lot of complaints that their Etherpad instances broke.

@SamTV12345
Copy link
Member

@HMarzban Do you have an idea how to fix the pipelines https://github.com/ether/etherpad-cli-client?

@HMarzban
Copy link
Contributor Author

HMarzban commented Feb 13, 2024

Sure, I'm quite busy at the moment, but I believe you can handle this.
If you need a detailed review, though, I'm here to help on Friday.

Just give these steps a try, and you should be able to fix it yourself!

Then, in this line:

  socket.json.send({
    type: 'COLLABROOM',
    component: 'pad',
    data: JSON.parse(JSON.stringify(padState.inFlight)),
  });

TO:

  socket.emit('message', {
    type: 'COLLABROOM',
    component: 'pad',
    data: JSON.parse(JSON.stringify(padState.inFlight)),
  });

Also, this line should be changed to:
socket.emit('message', msg);

@SamTV12345 SamTV12345 merged commit b2be2ca into ether:develop Feb 17, 2024
20 of 24 checks passed
@SamTV12345
Copy link
Member

Awesome. I had to go through a lot of hoops but eventually I managed to migrate the load test and the cli to socket.io v3. Thanks for the help with this upgrade and I am hyped for the upgrade to the latest socket io version

@HMarzban
Copy link
Contributor Author

Great, let's rock and roll. πŸ˜ƒ

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

Successfully merging this pull request may close these issues.

2 participants