Skip to content

Commit

Permalink
[FIX] fix connection error handling
Browse files Browse the repository at this point in the history
if connection to server can't be established, reject Promise of `connect()` method caller,
instead of throwing error event on base object.
  • Loading branch information
darkdarkdragon committed Feb 24, 2016
1 parent 7c9a179 commit 4acc42e
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 2 deletions.
20 changes: 18 additions & 2 deletions src/common/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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']
Expand All @@ -118,6 +127,10 @@ class Connection extends EventEmitter {
});
}

_onOpenError(reject, error) {
reject(new NotConnectedError(error && error.message));
}

_createWebSocket() {
const options = {};
if (this._proxyURL !== undefined) {
Expand Down Expand Up @@ -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.
Expand Down
20 changes: 20 additions & 0 deletions test/connection-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 4acc42e

Please sign in to comment.