Fixes 'Connection not open on send' #3171
Conversation
nivida
added
Bug
| _this.send(payload, callback); | ||
| }, 10); | ||
| this.connection.addEventListener( | ||
| 'open', |
There was a problem hiding this comment.
What happens if this is never fired because the client is not available, for example when testing against a local node the user forgot to turn on? In the current flow it looks like it will send and error, but here maybe hang?
There was a problem hiding this comment.
This is true it will with the changed lines of code just hang until the connection got established and in the case of a non-existing host will it never execute the JSON-RPC methods we have in the pipeline. The emitted error net::ERR_CONNECTION_REFUSED which would signal us the non-existing host. Can't get caught with a try/catch in the WebsocketProvider constructor because it doesn't throw an error in the normal sense. The WebSocket class does dispatch a non-listenable event because of the way it is implemented. A solution would be to create a connecting-timeout function in the constructor of the WebsocketProvider which throws an error if the connection doesn't get established in the defined time frame.
There was a problem hiding this comment.
A solution would be to create a connecting-timeout function in the constructor of the WebsocketProvider which throws an error if the connection doesn't get established in the defined time frame.
Great analysis! To me it seems like there are two execution contexts for Web3 with opposite expectations:
- NodeJS (testing/tools) --> fail fast and hard.
- Network (browser/dapps) --> keep trying for as long as it takes
At the moment, web3 behaves mostly the way NodeJS contexts would prefer, and there are lots of complaints on the browser / dapp side. Maybe we can just acknowledge the difference in the docs/release notes and address it by toggling the different behaviors via the autoReconnect: true (or something similar) proposed in #3167. Here you might just check that flag - stay open if true, fail fast as Web3 currently does if false?
WDYT? This change could be paired with #3167...
Co-Authored-By: cgewecke <[email protected]>
|
Moved this fix to #3190 |
Fixes #3092 with back-porting of the fix from 2.x
Instead of only waiting 10ms with a
setTimeoutand to try it again does it now register a listener on theopenevent of the underlying WebSocket object. I've configured the listener options withonce: trueto be sure the listener gets removed after it got invoked once.ToDo’s: