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
Assign event listener to socket close event on open before attempting post-open logic #1186
Assign event listener to socket close event on open before attempting post-open logic #1186
Conversation
… to execute post-connection logic, prottects possible unhandled rejection in disconnect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not that familiar with this code / WebSockets, so I'm going to see if I understand the problem here.
We handle cleanup in the _ws.close
event listener. However, previously we were setting up the _ws.close
listener too late, and we were getting an error after opening the connection, but before setting up the _ws.close
event listener responsible for cleanup.
That would result in subsequent reconnection errors, because we had failed to clean up before. Is that all correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments on the test, but overall this change is solid. Thanks again for filing & fixing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tested this out but it looks good to me
This commit adds the Connection
_ws.close
event listener post_ws.open
before executing any post_ws.open
logic, i.e.Connection._subscribeToLedger
. This prevents a reconnection error loop that occurs ifConnection._ws
is never cleaned up by the unreachable_ws.close
event listener. Also, this ensures that a possible disconnect() promise rejection is not unhandled if any_ws.open
logic inConnection.connect()
throws.