Skip to content

Commit

Permalink
Nikos/5835/websocket provider keeps important error message back (#5884)
Browse files Browse the repository at this point in the history
* Remove internal listener

* Clear ws connection manually

* Emit error for connectFailed instead of close

* Add test

* Add error reason and code, emit close, too

* Add description in error

* Remove connection listeners only on node

* Update changelog

* Check for client instead of connection for removing listener

* Remove console
  • Loading branch information
nikoulai authored Mar 6, 2023
1 parent ef23642 commit 2b3fb3a
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 4 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -655,5 +655,8 @@ Released with 1.0.0-beta.37 code base.
- Add optional `hexFormat` param to `getTransaction` and `getBlock` that accepts the value `'hex'` (#5845)
- `utils.toNumber` and `utils.hexToNumber` can now return the large unsafe numbers as `BigInt`, if `true` was passed to a new optional parameter called `bigIntOnOverflow` (#5845)
- Updated @types/bn.js dependency to 5.1.1 in web3, web3-core and web3-eth-contract as reason mentioned in #5640 (#5885)
- Add description to error for failed connection on websocket (#5884)


### Security
- Updated dependencies (#5885)
3 changes: 3 additions & 0 deletions packages/web3-core-helpers/src/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ module.exports = {
if (event) {
error.code = event.code;
error.reason = event.reason;
if(event.description) {
error.description = event.description;
}
}

return error;
Expand Down
54 changes: 50 additions & 4 deletions packages/web3-providers-ws/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ var WebsocketProvider = function WebsocketProvider(url, options) {
this.responseQueue = new Map();
this.reconnectAttempts = 0;
this.reconnecting = false;

this.connectFailedDescription = null;
// The w3cwebsocket implementation does not support Basic Auth
// username/password in the URL. So generate the basic auth header, and
// pass through with any additional headers supplied in constructor
Expand Down Expand Up @@ -160,6 +160,47 @@ WebsocketProvider.prototype._onConnect = function () {
}
};

WebsocketProvider.prototype._onConnectFailed = function (event) {
this.connectFailedDescription = event.toString().split('\n')[0];
var _this = this;
if (this.connectFailedDescription) {
event.description = this.connectFailedDescription;
this.connectFailedDescription = null; // clean the message, so it won't be used in the next connection
}

event.code = 1006;
event.reason = 'connection failed';

if (this.reconnectOptions.auto && (![1000, 1001].includes(event.code) || event.wasClean === false)) {
this.reconnect();

return;
}

this.emit(this.ERROR, event);
if (this.requestQueue.size > 0) {
this.requestQueue.forEach(function (request, key) {
request.callback(errors.ConnectionNotOpenError(event));
_this.requestQueue.delete(key);
});
}

if (this.responseQueue.size > 0) {
this.responseQueue.forEach(function (request, key) {
request.callback(errors.InvalidConnection('on WS', event));
_this.responseQueue.delete(key);
});
}

//clean connection on our own
if(this.connection._connection){
this.connection._connection.removeAllListeners();
}
this.connection._client.removeAllListeners();
this.connection._readyState = 3; // set readyState to CLOSED

this.emit(this.CLOSE, event);
}
/**
* Listener for the `close` event of the underlying WebSocket object
*
Expand All @@ -176,8 +217,7 @@ WebsocketProvider.prototype._onClose = function (event) {
return;
}

this.emit(this.CLOSE, event);

this.emit(this.CLOSE, event);
if (this.requestQueue.size > 0) {
this.requestQueue.forEach(function (request, key) {
request.callback(errors.ConnectionNotOpenError(event));
Expand Down Expand Up @@ -207,7 +247,11 @@ WebsocketProvider.prototype._addSocketListeners = function () {
this.connection.addEventListener('message', this._onMessage.bind(this));
this.connection.addEventListener('open', this._onConnect.bind(this));
this.connection.addEventListener('close', this._onClose.bind(this));
};
if(this.connection._client){
this.connection._client.removeAllListeners('connectFailed'); //Override the internal listeners, so they don't trigger a `close` event. We want to trigger `_onClose` manually with a description.
this.connection._client.on('connectFailed',this._onConnectFailed.bind(this));
}
}

/**
* Will remove all socket listeners
Expand All @@ -220,6 +264,8 @@ WebsocketProvider.prototype._removeSocketListeners = function () {
this.connection.removeEventListener('message', this._onMessage);
this.connection.removeEventListener('open', this._onConnect);
this.connection.removeEventListener('close', this._onClose);
if(this.connection._connection)
this.connection._client.removeListener('connectFailed',this._onConnectFailed);
};

/**
Expand Down
12 changes: 12 additions & 0 deletions test/websocket.ganache.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,18 @@ describe('WebsocketProvider (ganache)', function () {
web3.currentProvider.disconnect(1000);
})
})

it('"end" handler fires with close event object if Web3 disconnects', async function(){
this.timeout(5000);
server = ganache.server({ server: { ws: false, http: true } });
await server.listen(port);

const web3 = new Web3(new Web3.providers.WebsocketProvider(host + port));

web3.currentProvider.on('error',(err)=>{
assert(err.description.includes('Server responded with a non-101 status'));
})
})

// Here, the first error (try/caught) is fired by the request queue checker in
// the onClose handler. The second error is fired by the readyState check in .send
Expand Down

0 comments on commit 2b3fb3a

Please sign in to comment.