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

Bugfix/issue 27 invalid item access #29

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sam-cormack
Copy link

Fix for issue #27

Scheduled event handlers can run after a call to disconnect on the client. The conversation object has been cleared so item lookups cause an error to be thrown in some handlers.

With these changes, the event handler function returns early if the client is not connected, before trying to look up items.

A test case has been added, however the error triggers after the successful completion of the test. Not sure how to wait for all scheduled event handlers to run in the test.

@minons1
Copy link

minons1 commented Oct 10, 2024

cmiiw, I believe that isConnected() is not reflecting the status of the websocket connection, since when the websocket is closed without calling client.close() method, isConnected() will still resolve to true?

So the better way to check isConnected() is by validating the wsClient.readyState?

@sam-cormack
Copy link
Author

That's true, but it's the state of the conversation object that I'm really trying to check rather than the websocket connection. I don't think there's any issue if the event handlers run after the websocket disconnects (though I could be wrong about that). conversation.clear() is only called alongside realtime.disconnect() at the moment so checking realtime.isConnected() does the job. It's probably not ideal, since conversation.clear() could get called elsewhere - even from client code.

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