From d70acd946278e2583290df40ffa271c4856ebb67 Mon Sep 17 00:00:00 2001 From: Gabriel Montes Date: Wed, 5 Sep 2018 10:10:21 -0300 Subject: [PATCH 01/11] Attach ws event listeners using EventTarget API When using `.on=fn` to attach listeners, only one listener can be set at the same time. Since multiple request managers can use the same provider, the EventTarget API has to be used to ensure all of them receive the events emitted from the provider. This is needed on both the `on` and `removeListener` functions. --- packages/web3-providers-ws/src/index.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/web3-providers-ws/src/index.js b/packages/web3-providers-ws/src/index.js index b9a271f5cd6..44d50a37c69 100644 --- a/packages/web3-providers-ws/src/index.js +++ b/packages/web3-providers-ws/src/index.js @@ -305,15 +305,15 @@ WebsocketProvider.prototype.on = function (type, callback) { break; case 'connect': - this.connection.onopen = callback; + this.connection.addEventListener('open', callback); break; case 'end': - this.connection.onclose = callback; + this.connection.addEventListener('close', callback); break; case 'error': - this.connection.onerror = callback; + this.connection.addEventListener('error', callback); break; // default: @@ -342,7 +342,17 @@ WebsocketProvider.prototype.removeListener = function (type, callback) { }); break; - // TODO remvoving connect missing + case 'connect': + this.connection.removeEventListener('open', callback); + break; + + case 'end': + this.connection.removeEventListener('close', callback); + break; + + case 'error': + this.connection.removeEventListener('error', callback); + break; // default: // this.connection.removeListener(type, callback); From ebd252c3e4a89b3e42c3c995b9998998fadf45c4 Mon Sep 17 00:00:00 2001 From: Gabriel Montes Date: Wed, 5 Sep 2018 10:13:28 -0300 Subject: [PATCH 02/11] Add `once` to the WS provider interface The method `once` is required to allow the subscription logic to identify if the provider is able to reconnect/resubscribe and then attach to the following `connect` event the function to resubscribe. --- packages/web3-providers-ws/src/index.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/web3-providers-ws/src/index.js b/packages/web3-providers-ws/src/index.js index 44d50a37c69..4d9d12e7918 100644 --- a/packages/web3-providers-ws/src/index.js +++ b/packages/web3-providers-ws/src/index.js @@ -322,7 +322,24 @@ WebsocketProvider.prototype.on = function (type, callback) { } }; -// TODO add once +/** + Subscribes to provider only once + + @method once + @param {String} type 'notifcation', 'connect', 'error', 'end' or 'data' + @param {Function} callback the callback to call + */ +WebsocketProvider.prototype.once = function (type, callback) { + var _this = this; + + function onceCallback(event) { + _this.removeListener(type, onceCallback); + + callback(event); + } + + this.on(type, onceCallback); +}; /** Removes event listener From 9765b1aa688a534d983cf70ef527e75fe4174a4c Mon Sep 17 00:00:00 2001 From: Gabriel Montes Date: Wed, 5 Sep 2018 10:25:47 -0300 Subject: [PATCH 03/11] Merge logic for resubscription When the subscription fails on start and when it fails after it was successfully established, the same logic needs to be executed: remove subscription, listen for the next `connect` event if available to actually subscribe again, emit the error and call the callback. Prior code did that only for established subscriptions so if a subscription was unable to be set right on start, no resubscription was ever tried. The logic was moved to a single method to avoid duplication of code. In addition reentry is avoided by checking and properly clearing the `_reconnectIntervalId` variable. --- .../src/subscription.js | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/packages/web3-core-subscriptions/src/subscription.js b/packages/web3-core-subscriptions/src/subscription.js index 92d24551b45..a977f967082 100644 --- a/packages/web3-core-subscriptions/src/subscription.js +++ b/packages/web3-core-subscriptions/src/subscription.js @@ -272,37 +272,44 @@ Subscription.prototype.subscribe = function() { _this.callback(null, output, _this); }); } else { - // unsubscribe, but keep listeners - _this.options.requestManager.removeSubscription(_this.id); + _this._resubscribe(err); + } + }); + } else { + _this._resubscribe(err); + } + }); + + // return an object to cancel the subscription + return this; +}; + +Subscription.prototype._resubscribe = function (err) { + var _this = this; + + // unsubscribe + this.options.requestManager.removeSubscription(this.id); // re-subscribe, if connection fails - if(_this.options.requestManager.provider.once) { - _this._reconnectIntervalId = setInterval(function () { + if(this.options.requestManager.provider.once && !_this._reconnectIntervalId) { + this._reconnectIntervalId = setInterval(function () { // TODO check if that makes sense! if (_this.options.requestManager.provider.reconnect) { _this.options.requestManager.provider.reconnect(); } }, 500); - _this.options.requestManager.provider.once('connect', function () { + this.options.requestManager.provider.once('connect', function () { clearInterval(_this._reconnectIntervalId); + _this._reconnectIntervalId = null; _this.subscribe(_this.callback); }); } - _this.emit('error', err); - // call the callback, last so that unsubscribe there won't affect the emit above - _this.callback(err, null, _this); - } - }); - } else { - _this.callback(err, null, _this); - _this.emit('error', err); - } - }); + this.emit('error', err); - // return an object to cancel the subscription - return this; + // call the callback, last so that unsubscribe there won't affect the emit above + this.callback(err, null, this); }; module.exports = Subscription; From c45792fbc75861421728f2d1d1c1f78908f5b79c Mon Sep 17 00:00:00 2001 From: Gabriel Montes Date: Wed, 5 Sep 2018 10:32:01 -0300 Subject: [PATCH 04/11] Clear subscription id before resubscribing On subscribe, if there is an existing `id`, the subscription listeners are removed. In the case of a resubscription, the listeners have to be kept. Therefore, the `id` property -that will change anyway- must be cleared so the listeners are not removed. Then, after the subscription object resubscribes, the listeners set by the subscription user code remain untouched, making the resubscription transparent to the user code. --- .../src/subscription.js | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/web3-core-subscriptions/src/subscription.js b/packages/web3-core-subscriptions/src/subscription.js index a977f967082..b59160b721f 100644 --- a/packages/web3-core-subscriptions/src/subscription.js +++ b/packages/web3-core-subscriptions/src/subscription.js @@ -290,25 +290,29 @@ Subscription.prototype._resubscribe = function (err) { // unsubscribe this.options.requestManager.removeSubscription(this.id); - // re-subscribe, if connection fails + // re-subscribe, if connection fails if(this.options.requestManager.provider.once && !_this._reconnectIntervalId) { this._reconnectIntervalId = setInterval(function () { - // TODO check if that makes sense! - if (_this.options.requestManager.provider.reconnect) { - _this.options.requestManager.provider.reconnect(); - } - }, 500); + // TODO check if that makes sense! + if (_this.options.requestManager.provider.reconnect) { + _this.options.requestManager.provider.reconnect(); + } + }, 500); this.options.requestManager.provider.once('connect', function () { - clearInterval(_this._reconnectIntervalId); + clearInterval(_this._reconnectIntervalId); _this._reconnectIntervalId = null; - _this.subscribe(_this.callback); - }); - } + + // delete id to keep the listeners on subscribe + _this.id = null; + + _this.subscribe(_this.callback); + }); + } this.emit('error', err); - // call the callback, last so that unsubscribe there won't affect the emit above + // call the callback, last so that unsubscribe there won't affect the emit above this.callback(err, null, this); }; From 2ceae25f5a74c981b97977ecabf38979126af788 Mon Sep 17 00:00:00 2001 From: Gabriel Montes Date: Wed, 5 Sep 2018 10:36:11 -0300 Subject: [PATCH 05/11] Avoid reentry when removing subscriptions When the request manager removes a subscription due to an error, it tries to send an unsubscribe package, which can also fail if i.e. the network is down. In such a case, the function must not allow reentry. Removing the subscription first ensures it will not do so. In addition, if the subscription was already removed, the callback shall be called anyway. --- packages/web3-core-requestmanager/src/index.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/web3-core-requestmanager/src/index.js b/packages/web3-core-requestmanager/src/index.js index 4fc48d493c7..212a9d333fb 100644 --- a/packages/web3-core-requestmanager/src/index.js +++ b/packages/web3-core-requestmanager/src/index.js @@ -205,17 +205,20 @@ RequestManager.prototype.addSubscription = function (id, name, type, callback) { * @param {Function} callback fired once the subscription is removed */ RequestManager.prototype.removeSubscription = function (id, callback) { - var _this = this; - if(this.subscriptions[id]) { + var type = this.subscriptions[id].type; + // remove subscription first to avoid reentry + delete this.subscriptions[id]; + + // then, try to actually unsubscribe this.send({ - method: this.subscriptions[id].type + '_unsubscribe', + method: type + '_unsubscribe', params: [id] }, callback); - - // remove subscription - delete _this.subscriptions[id]; + } else if (typeof callback === 'function') { + // call the callback if the subscription was already removed + callback(null); } }; From 269ecebfe828823a77943db2f150bba509a7d158 Mon Sep 17 00:00:00 2001 From: Gabriel Montes Date: Wed, 5 Sep 2018 10:39:41 -0300 Subject: [PATCH 06/11] Broadcast provider error to subscribers When error events are emitted by the provider, all subscriptions shall receive the event and trigger the unsubscription/resubscription logic. --- packages/web3-core-requestmanager/src/index.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/web3-core-requestmanager/src/index.js b/packages/web3-core-requestmanager/src/index.js index 212a9d333fb..d07c0cd9fb0 100644 --- a/packages/web3-core-requestmanager/src/index.js +++ b/packages/web3-core-requestmanager/src/index.js @@ -103,13 +103,16 @@ RequestManager.prototype.setProvider = function (p, net) { _this.subscriptions[result.params.subscription].callback(null, result.params.result); } }); - // TODO add error, end, timeout, connect?? - // this.provider.on('error', function requestManagerNotification(result){ - // Object.keys(_this.subscriptions).forEach(function(id){ - // if(_this.subscriptions[id].callback) - // _this.subscriptions[id].callback(err); - // }); - // } + + // notify all subscriptions about the error condition + this.provider.on('error', function (event) { + Object.keys(_this.subscriptions).forEach(function(id){ + if(_this.subscriptions[id] && _this.subscriptions[id].callback) + _this.subscriptions[id].callback(event.code || new Error('Provider error')); + }); + }); + + // TODO add end, timeout, connect?? } }; From e2fa94af4c28c233b095ad2d3b1022e4badbd3de Mon Sep 17 00:00:00 2001 From: Gabriel Montes Date: Thu, 6 Sep 2018 10:57:54 -0300 Subject: [PATCH 07/11] Add support for WebSocket reconnections By wrapping the available WebSocket implementation (native WebSocket object or `websocket` package) with `websocket-reconnector`, the provider is given a WebSocket that will automatically reconnect on errors. A new option was added to the WebSocket provider to controll whether it should automatically reconnect or it should behave as usual. --- packages/web3-providers-ws/README.md | 5 +++-- packages/web3-providers-ws/package.json | 5 +++-- packages/web3-providers-ws/src/index.js | 6 ++++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/web3-providers-ws/README.md b/packages/web3-providers-ws/README.md index 4be274764be..3cf2c7b640a 100644 --- a/packages/web3-providers-ws/README.md +++ b/packages/web3-providers-ws/README.md @@ -32,8 +32,9 @@ var Web3WsProvider = require('web3-providers-ws'); var options = { timeout: 30000, - headers: { authorization: 'Basic username:password' } -}; // set a custom timeout at 30 seconds, and credentials (you can also add the credentials to the URL: ws://username:password@localhost:8546) + headers: { authorization: 'Basic username:password' }, + autoReconnect: true +}; // set a custom timeout at 30 seconds, credentials (you can also add the credentials to the URL: ws://username:password@localhost:8546), and enable WebSocket auto-reconnection var ws = new Web3WsProvider('ws://localhost:8546', options); ``` diff --git a/packages/web3-providers-ws/package.json b/packages/web3-providers-ws/package.json index 50519e7d119..7d916a550ac 100644 --- a/packages/web3-providers-ws/package.json +++ b/packages/web3-providers-ws/package.json @@ -15,10 +15,11 @@ "dependencies": { "underscore": "1.9.1", "web3-core-helpers": "1.2.2", - "websocket": "github:web3-js/WebSocket-Node#polyfill/globalThis" + "websocket": "github:web3-js/WebSocket-Node#polyfill/globalThis", + "websocket-reconnector": "1.1.1" }, "devDependencies": { "definitelytyped-header-parser": "^1.0.1", "dtslint": "0.4.2" } -} +} \ No newline at end of file diff --git a/packages/web3-providers-ws/src/index.js b/packages/web3-providers-ws/src/index.js index 4d9d12e7918..b10a631bdb1 100644 --- a/packages/web3-providers-ws/src/index.js +++ b/packages/web3-providers-ws/src/index.js @@ -25,6 +25,7 @@ var _ = require('underscore'); var errors = require('web3-core-helpers').errors; var Ws = require('websocket').w3cwebsocket; +var WsReconnector = require('websocket-reconnector'); var isNode = Object.prototype.toString.call(typeof process !== 'undefined' ? process : 0) === '[object process]'; @@ -86,6 +87,11 @@ var WebsocketProvider = function WebsocketProvider(url, options) { // https://github.com/theturtle32/WebSocket-Node/blob/master/docs/WebSocketClient.md#connectrequesturl-requestedprotocols-origin-headers-requestoptions var requestOptions = options.requestOptions || undefined; + // Enable automatic reconnection wrapping `Ws` with reconnector + if (options.autoReconnect) { + Ws = WsReconnector(Ws); + } + // When all node core implementations that do not have the // WHATWG compatible URL parser go out of service this line can be removed. if (parsedURL.auth) { From bef337e6b64570c8c0887594e6ce0ff6ad27f21a Mon Sep 17 00:00:00 2001 From: Gabriel Montes Date: Thu, 6 Sep 2018 20:31:50 -0300 Subject: [PATCH 08/11] Try to reconnect on timeout too In the case any websocket call takes too long to return and a timeout was set for the provider to timeout, the provider should try to restart the connection. This could happen, for instance, if the client loses connection with the server, the server closes the connection and later, the connectivity is up again but since the client did not receive the closing frame *and* the client does not attempt to send any package to the server, no error is observed. `websocket` implementation for Node.js has an option to send keep-alive frames and detect such scenarios, but the standard browser W3C WebSocket does not, so it is "vulnerable" to this kind of failure which will mostly affect web3 subscriptions. --- packages/web3-providers-ws/src/index.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/web3-providers-ws/src/index.js b/packages/web3-providers-ws/src/index.js index b10a631bdb1..086a5e34fc5 100644 --- a/packages/web3-providers-ws/src/index.js +++ b/packages/web3-providers-ws/src/index.js @@ -244,7 +244,13 @@ WebsocketProvider.prototype._addResponseCallback = function(payload, callback) { setTimeout(function () { if (_this.responseCallbacks[id]) { _this.responseCallbacks[id](errors.ConnectionTimeout(_this._customTimeout)); + delete _this.responseCallbacks[id]; + + // try to reconnect + if (_this.connection.reconnect) { + _this.connection.reconnect(); + } } }, this._customTimeout); } From 20b5569b90003fbcdbafcad4665e9113320d5435 Mon Sep 17 00:00:00 2001 From: Gabriel Montes Date: Mon, 7 Oct 2019 16:04:04 -0300 Subject: [PATCH 09/11] Remove WS provider once listener on next tick --- packages/web3-providers-ws/src/index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/web3-providers-ws/src/index.js b/packages/web3-providers-ws/src/index.js index 086a5e34fc5..df4c7ff0e74 100644 --- a/packages/web3-providers-ws/src/index.js +++ b/packages/web3-providers-ws/src/index.js @@ -345,7 +345,9 @@ WebsocketProvider.prototype.once = function (type, callback) { var _this = this; function onceCallback(event) { - _this.removeListener(type, onceCallback); + setTimeout(function () { + _this.removeListener(type, onceCallback); + }, 0); callback(event); } From e556299b441bce95505ab662e7fcf1a5ab492f4f Mon Sep 17 00:00:00 2001 From: Gabriel Montes Date: Mon, 7 Oct 2019 16:06:28 -0300 Subject: [PATCH 10/11] Ensure resubscription is done on silent reconnects If the provider silently recoonects and emits a new "connect" event, the subscriptions have to be set again over that new connection. --- packages/web3-core-subscriptions/src/subscription.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/web3-core-subscriptions/src/subscription.js b/packages/web3-core-subscriptions/src/subscription.js index b59160b721f..9ec301f1615 100644 --- a/packages/web3-core-subscriptions/src/subscription.js +++ b/packages/web3-core-subscriptions/src/subscription.js @@ -275,6 +275,13 @@ Subscription.prototype.subscribe = function() { _this._resubscribe(err); } }); + + // just in case the provider reconnects silently, resubscribe over the new connection + if (_this.options.requestManager.provider.once) { + _this.options.requestManager.provider.once('connect', function () { + _this._resubscribe(); + }); + } } else { _this._resubscribe(err); } @@ -310,7 +317,9 @@ Subscription.prototype._resubscribe = function (err) { }); } - this.emit('error', err); + if (err) { + this.emit('error', err); + } // call the callback, last so that unsubscribe there won't affect the emit above this.callback(err, null, this); From 66f7f74e99a8bd3e3a04309e96add12e87b2ba8f Mon Sep 17 00:00:00 2001 From: Gabriel Montes Date: Tue, 22 Oct 2019 09:22:23 -0300 Subject: [PATCH 11/11] Fix indentation --- packages/web3-providers-ws/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web3-providers-ws/src/index.js b/packages/web3-providers-ws/src/index.js index df4c7ff0e74..8501a2fd28d 100644 --- a/packages/web3-providers-ws/src/index.js +++ b/packages/web3-providers-ws/src/index.js @@ -250,7 +250,7 @@ WebsocketProvider.prototype._addResponseCallback = function(payload, callback) { // try to reconnect if (_this.connection.reconnect) { _this.connection.reconnect(); - } + } } }, this._customTimeout); }