Closed nickewansmith closed 4 years ago
Here is a proposed solution https://github.com/ripple/ripple-lib/pull/1186
hey @nickewansmith, thanks for filing this! This logic is definitely tricky, so I want to make sure I understand your issue & the reproduction example and give some context:
on('close')
logic was written to clean up a live Connection, that may have requests in flight. _subscribeToLedger()
step was written to happen before connect()
resolved, meaning there would never be live/in-flight connections needing to be cleaned up at this point.connect()
should only resolve once _subscribeToLedger()
resolves._subscribeToLedger()
rejects, connect()
should also reject. _subscribeToLedger()
hangs, thats a bug since it should never hang past a timeout :)_subscribeToLedger()
rejects (like in your example above) it's error should be caught in that try/catch, where we then call disconnect()
which handles the _ws
cleanup via this._ws.close()
: https://github.com/ripple/ripple-lib/blob/develop/src/common/connection.ts#L538Does that make sense? If I'm missing something / you are still seeing unexpected behavior, let me know.
Hey, np @FredKSchott. What you've explained is correct, however, because the try/catch in question is run before the _ws.on('close')
event gets a listener, calling _ws.close()
in the disconnect()
run in the catch never runs a cleanup listener for the close event, never cleaning up the connection and causing errors on each subsequent attempt to connect Websocket connection never cleaned up
.
Ah, you're right! Thanks for filling in that missing bit.
Okay, let's discuss solutions in #1186 since I think that PR is on the right track.
It seems that if Connection
_subscribeToLedger
throws onconnect()
that Connection_ws
is never cleaned up due to the cleanup code inconnect()
->_ws.once('close')
never actually being reached due to returning immediately on throw.This can be reproduced in this way: