From 4acc42e1b653cb48e43ad7ead14be96251ab3c44 Mon Sep 17 00:00:00 2001 From: Ivan Tivonenko Date: Wed, 24 Feb 2016 06:09:19 +0200 Subject: [PATCH] [FIX] fix connection error handling if connection to server can't be established, reject Promise of `connect()` method caller, instead of throwing error event on base object. --- src/common/connection.js | 20 ++++++++++++++++++-- test/connection-test.js | 20 ++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/common/connection.js b/src/common/connection.js index b39bebe7..e8ad4af3 100644 --- a/src/common/connection.js +++ b/src/common/connection.js @@ -97,6 +97,10 @@ class Connection extends EventEmitter { } _onUnexpectedClose(resolve = function() {}, reject = function() {}) { + if (this._onOpenErrorBound) { + this._ws.removeListener('error', this._onOpenErrorBound); + this._onOpenErrorBound = null; + } this._ws = null; this._isReady = false; this.connect().then(resolve, reject); @@ -107,6 +111,11 @@ class Connection extends EventEmitter { this._onUnexpectedCloseBound = this._onUnexpectedClose.bind(this); this._ws.once('close', this._onUnexpectedCloseBound); + this._ws.removeListener('error', this._onOpenErrorBound); + this._onOpenErrorBound = null; + this._ws.on('error', error => + this.emit('error', 'websocket', error.message, error)); + const request = { command: 'subscribe', streams: ['ledger'] @@ -118,6 +127,10 @@ class Connection extends EventEmitter { }); } + _onOpenError(reject, error) { + reject(new NotConnectedError(error && error.message)); + } + _createWebSocket() { const options = {}; if (this._proxyURL !== undefined) { @@ -177,8 +190,11 @@ class Connection extends EventEmitter { // should still be emitted; the "ws" documentation says: "The close // event is also emitted when then underlying net.Socket closes the // connection (end or close)." - this._ws.on('error', error => - this.emit('error', 'websocket', error.message, error)); + // In case if there is connection error (say, server is not responding) + // we must return this error to connection's caller. After successful + // opening, we will forward all errors to main api object. + this._onOpenErrorBound = this._onOpenError.bind(this, reject); + this._ws.once('error', this._onOpenErrorBound); this._ws.on('message', this._onMessage.bind(this)); // in browser close event can came before open event, so we must // resolve connect's promise after reconnect in that case. diff --git a/test/connection-test.js b/test/connection-test.js index bbf360bb..10c88efc 100644 --- a/test/connection-test.js +++ b/test/connection-test.js @@ -106,6 +106,26 @@ describe('Connection', function() { }); }); + it('should throw NotConnectedError if server not responding ', function( + done + ) { + if (process.browser) { + const phantomTest = /PhantomJS/; + if (phantomTest.test(navigator.userAgent)) { + // inside PhantomJS this one just hangs, so skip as not very relevant + done(); + return; + } + } + + const connection = new utils.common.Connection('ws://127.0.0.1:321'); + connection.on('error', done); + connection.connect().catch(error => { + assert(error instanceof this.api.errors.NotConnectedError); + done(); + }); + }); + it('DisconnectedError', function() { this.api.connection._send(JSON.stringify({ command: 'config',