fix: socket.io upgrade sometimes happen before connect

[sandangel]

Signed-off-by: San Nguyen vinhsannguyen91@gmail.com

[rohanbalkondekar]

@CodiumAI-Agent /review

[CodiumAI-Agent]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Race Condition The handling of the 'upgradeError' and 'connect' events could lead to race conditions or multiple reconnections due to the asynchronous nature of these events. This might result in unexpected behavior or performance issues.

[rohanbalkondekar]

@CodiumAI-Agent /describe

[CodiumAI-Agent]

Title

(Describe updated until commit 6ea3ac0)

fix: socket.io upgrade sometimes happen before connect

---

User description

Signed-off-by: San Nguyen vinhsannguyen91@gmail.com

---

PR Type

Bug fix

---

Description

• Fixed a bug where the WebSocket upgrade could occur before the connect event, causing issues with user interaction.
• Simplified the event handling by integrating the 'upgrade' and 'connect' events.
• Improved error handling by directly referencing socket.io.engine for upgrade errors.

---

Changes walkthrough 📝

Relevant files

Bug fix

useChatSession.ts Fix WebSocket upgrade timing issue in useChatSession          libs/react-client/src/useChatSession.ts Fixed the timing issue where the upgrade to WebSocket could happen before the connect event. Removed the separate handling of the 'upgrade' event and integrated it with the 'connect' event. Added a check to ensure the WebSocket transport is used before invoking the onConnect function. Simplified the error handling for upgrade errors by directly referencing socket.io.engine. +12/-16

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[sandangel]

Hi, may I ask any update to this PR?

[dokterbob]

Hi, may I ask any update to this PR?

The description of your PR was empty, so it's hard for us to review it.

Could you please describe in detail:

1. What specific use case(s) it addresses or, preferably, how we might replicate this issue.
2. What the expected end result is.
3. What happens instead?
4. What reasoning led to the specific proposed solution, including possible alternatives you have considered.

Lastly, why did you remove the comments there? As this is quite a particular issue, what's the need of not having a direct reference to the context at hand?

[sandangel]

@dokterbob hi, I think I removed those comments because they were not correct.
These functions are based on socket.io events, but there are race condition when we have listeners nested in other listeners. So the initial implementation was not correct.
Another issue we have is the socket.io.engine is a different instance when the connection is established, so we should just reference the socket.io.engine directly without assigning variable to it.

[sandangel]

Hi, I guess we might just remove the force websocket mode things together. It might not work as intended, hard to test. Any opinion on this?

[sandangel]

Closing. I will open a PR to revert the require websocket that I added before.