Skip to content

Conversation

@StrongMonkey
Copy link
Contributor

This PR fixes an issue where socket's event emitter will be lost once socket is reconnected. This happens when app lost network connection, causing socket to reconnect but the server socket is reconnected and lose all the event listener including user-message. This causes an issue where typing user message doesn't trigger anything in UI.

Added the functionality to manually trigger the run event through the socket to re-add all the custom event listeners.

Signed-off-by: Daishan Peng <daishan@acorn.io>

useEffect(() => {
if (forceRun && socket && connected) {
socket.emit("run", script, subTool ? subTool : tool.name, formValues, workspace, thread)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tylerslaton Let me know how you would think about the approach... this is the best way I can think of to trigger the socket re-mount logic during reconnection

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a bad approach. I wonder if you could call the restart function from the useChatSocket.tsx here instead since that'll wire a bunch of stuff up on your behalf. That being said, I think what you have works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, trying to avoid the fact that when reconnecting socket we just create a new one. I do feel like the proper solution to handle this in the server side where socket should listen on connect event and remount all the listener, but that's much harder because there are a bunch of paramaters to be passed in when mounting the listener.


useEffect(() => {
if (forceRun && socket && connected) {
socket.emit("run", script, subTool ? subTool : tool.name, formValues, workspace, thread)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a bad approach. I wonder if you could call the restart function from the useChatSocket.tsx here instead since that'll wire a bunch of stuff up on your behalf. That being said, I think what you have works.

@StrongMonkey StrongMonkey merged commit 2a2d358 into gptscript-ai:main Aug 11, 2024
@StrongMonkey
Copy link
Contributor Author

#119

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