WIP: Socket.io v3

[HMarzban]

[JohnMcLear]

Excited to watch the tests run 👯‍♀️

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 2d08f62e89d316872bbeaaf3029e0d69a8b2cd15 into 0aad3b74da7ab9aa2cf560a42cd592a7a4c9b40b - view on LGTM.com

new alerts:

• 1 for Unneeded defensive code

[HMarzban]

If setting.loadTest: true, in most cases the test will fail! ( webaccess issue) The userCanModify and checkAccess functions in /hooks/express/webaccess.js file, need to modify!

Also, websocket, chat, etc. tests passed successfully! But on the client side, when the user makes new changes, the other client will not receive new changes! (This is the main issue right now)

[lgtm-com[bot]]

This pull request introduces 1 alert when merging e1d8b415e541bf412f4543f1c2729166569aaf37 into 0aad3b74da7ab9aa2cf560a42cd592a7a4c9b40b - view on LGTM.com

new alerts:

• 1 for Unneeded defensive code

[HMarzban]

From now on, we have to set the transports option on the client and server to io.connect (opt)! (This branch; src/node/hooks/express/socketio.js:73)

In this case we must have to put settings.socketTransportProtocols in clientVars for "pad.html" (and where necessary) when it's render in the back-end.

[JohnMcLear]

If setting.loadTest: true, in most cases the test will fail! ( webaccess issue) The userCanModify and checkAccess functions in /hooks/express/webaccess.js file, need to modify!

Also, websocket, chat, etc. tests passed successfully! But on the client side, when the user makes new changes, the other client will not receive new changes! (This is the main issue right now)

This is because the tests mostly don't cover real time collaboration, this is something I'm working on at the moment :)

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 7fc14d7088ab1949406dd76dc654af84b44359e7 into 912f0f195faf19b11a5db928b3846fbb09388004 - view on LGTM.com

new alerts:

• 1 for Unneeded defensive code

[HMarzban]

Well, let me drop the mic, the real time collaboration issue was resolved. work like magic ⚔🧝‍♂️

If setting.loadTest: true, in most cases the test will fail! ( webaccess issue) The userCanModify and checkAccess functions in /hooks/express/webaccess.js file, need to modify! Also, websocket, chat, etc. tests passed successfully! But on the client side, when the user makes new changes, the other client will not receive new changes! (This is the main issue right now)

This is because the tests mostly don't cover real time collaboration, this is something I'm working on at the moment :)

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 2f4ef59554a7edb89da829816583f6becb0d080c into 912f0f195faf19b11a5db928b3846fbb09388004 - view on LGTM.com

new alerts:

• 1 for Unneeded defensive code

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 5b4c1d97202e643e689a793876bd9e38ca25b870 into 912f0f195faf19b11a5db928b3846fbb09388004 - view on LGTM.com

new alerts:

• 1 for Unneeded defensive code

[lgtm-com[bot]]

This pull request introduces 1 alert when merging b39e0419dd219ad4472a81702e5b6645cd736ab2 into 4ca989a255a9b4eed06b122b9fad97304eb930d7 - view on LGTM.com

new alerts:

• 1 for Unneeded defensive code

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 5396be1d297d1a3cdcdf882026f4892098e5371e into 4ca989a255a9b4eed06b122b9fad97304eb930d7 - view on LGTM.com

new alerts:

• 1 for Unneeded defensive code

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 6af8564fa1778bd56f0e9a4d3ab3463de6d50081 into 404486069c204cbec840aa802fc6a18dac013418 - view on LGTM.com

new alerts:

• 1 for Unneeded defensive code

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 916b2d8143743ee8b906c01a5becfd6697b26e9f into 404486069c204cbec840aa802fc6a18dac013418 - view on LGTM.com

new alerts:

• 1 for Unneeded defensive code

[JohnMcLear]

This works enough for me to run the load tests, and it does appear that the impact on Etherpad is negligible so this wont help much in achieving additional authors on a pad.

Our next step should be:

1. Confirming it's still socketIO creating the top load (I'm 99% sure it will but I'd like fresh eyes to help profile).
2. Look to migrate to websocket and drop socketio.

My conclusion here is that SocketIO is more of a burden now than a gain.

[JohnMcLear]

Note that we will need to ensure padId is still kept within the query string as per https://github.com/ether/etherpad-lite/pull/4793

[HMarzban]

I was reading this changeset recently: https://socket.io/blog/monthly-update-3/#Socket-IO-v4

If we do this migration, The road is paved for us for further upgrades

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 2e29f2301b7680b9b5b94e41e1d5985c64908f56 into a7968115581e20ef47a533e030f59f830486bdfa - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class
• 1 for Unneeded defensive code

[HMarzban]

@webzwo0i Sorry to be late for comments, I was in vacation.

[lgtm-com[bot]]

This pull request introduces 1 alert when merging f516d320a13093f470007e3d68327254abed32cb into a7968115581e20ef47a533e030f59f830486bdfa - view on LGTM.com

new alerts:

• 1 for Unneeded defensive code

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[HMarzban]

Hey guys, have you thought about this PR! I have already fixed all the problems related to this upgrade! do you have a socket upgrade plan or is it necessary anymore? @JohnMcLear @webzwo0i

[rhansen]

Before we can merge this we need to figure out a way to support plugins that expect socket.io 2.x. For example: https://github.com/ether/ep_webrtc/blob/7e5941f4a21ab869a8448ecb771c844643f3ef94/index.js#L46-L48

[HMarzban]

It's okay, I can help you! I change this function like below and it works like a charm!

const handleRTCMessage = (socket, client, payload) => {
  // if(!socketIo) return false
  const userId = payload.from;
  const padId = payload.padId;
  const to = payload.to;

  const msg = {
    type: 'COLLABROOM',
    data: {
      type: 'RTC_MESSAGE',
      payload: {
        from: userId,
        to,
        data: payload.data,
      },
    },
  };
  socketIo.to(padId).emit('RTC_MESSAGE', msg);
};

[rhansen]

My point is that we need to audit all plugins to determine which ones depend on v2-specific behavior, then update them to work both with and without this PR. Alternatively, change this PR so that the plugins that depend on v2-specific behavior somehow continue to work unchanged.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[SamTV12345]

@HMarzban I know it's been some time since you worked on this pull request. During the last 2.5 years has changed and I am currently the only maintainer of Etherpad. I am looking for help with the upgrade to socket.io v4. So if you find some time it would be awesome if you could help me with the upgrade. With the help of @JohnMcLear I managed to revive Etherpad, made lots of changes to Etherpad and introduced Typescript to ueberdb. If you prepare a pull request you can be sure that it will be merged. I normally answer in at most one day.

So TDLR: It would be awesome if you could help me with the socket io v4 upgrade.

[HMarzban]

Hi, @SamTV12345

Thank you for reaching out and for keeping Etherpad thriving! It's great to hear about the progress and the introduction of TypeScript to ueberdb.

I'm excited to help with the socket.io v4 upgrade. (It was a quick, challenging experience last time, but I did it and it worked!)

However, I'm currently swamped with tasks and will be busy until the 2nd of February. But right after that, I'm all in! I'm looking forward to diving back into the wavy, turbulent ocean of Etherpad code and exploring the best paths for the upgrade. (I appreciate your patience.)

[SamTV12345]

Hi, @SamTV12345

Thank you for reaching out and for keeping Etherpad thriving! It's great to hear about the progress and the introduction of TypeScript to ueberdb.

I'm excited to help with the socket.io v4 upgrade. (It was a quick, challenging experience last time, but I did it and it worked!)

However, I'm currently swamped with tasks and will be busy until the 2nd of February. But right after that, I'm all in! I'm looking forward to diving back into the wavy, turbulent ocean of Etherpad code and exploring the best paths for the upgrade. (I appreciate your patience.)

Thanks for the help. This is very much appreciated. Yeah the Etherpad code with its hundreds of public methods is a bit overwhelming at least for me. So any help is awesome. Sure it is not a problem if you are currently busy. So until February and thanks for reporting back.

[SamTV12345]

I guess we can finally close this <3