From e17b6f172d9efa76dd6924031c4f528f47d8742b Mon Sep 17 00:00:00 2001 From: Ivan Tivonenko Date: Thu, 24 Mar 2016 22:09:49 +0200 Subject: [PATCH] [FIX] on connect return error if server doesn't have validated ledgers --- src/api.js | 4 +- src/common/connection.js | 41 ++++++++++---- src/common/serverinfo.js | 28 +++++----- test/api-test.js | 15 +++++- test/connection-test.js | 54 +++++++++++++++++++ test/fixtures/rippled/empty.json | 7 +++ test/fixtures/rippled/index.js | 4 +- .../rippled/server-info-no-validated.json | 31 +++++++++++ test/mock-rippled.js | 23 +++++++- 9 files changed, 177 insertions(+), 30 deletions(-) create mode 100644 test/fixtures/rippled/empty.json create mode 100644 test/fixtures/rippled/server-info-no-validated.json diff --git a/src/api.js b/src/api.js index 9cd635af..f93c9019 100644 --- a/src/api.js +++ b/src/api.js @@ -92,8 +92,8 @@ class RippleAPI extends EventEmitter { this.connection.on('connected', () => { this.emit('connected'); }); - this.connection.on('disconnected', onError => { - this.emit('disconnected', onError); + this.connection.on('disconnected', code => { + this.emit('disconnected', code); }); } else { // use null object pattern to provide better error message if user diff --git a/src/common/connection.js b/src/common/connection.js index 42c92ab1..8272d92d 100644 --- a/src/common/connection.js +++ b/src/common/connection.js @@ -146,25 +146,36 @@ class Connection extends EventEmitter { } _onOpen() { - this._ws.removeListener('close', this._onUnexpectedCloseBound); - this._onUnexpectedCloseBound = - this._onUnexpectedClose.bind(this, false, null, null); - this._ws.once('close', this._onUnexpectedCloseBound); - this._ws.removeListener('error', this._onOpenErrorBound); this._onOpenErrorBound = null; - this._retry = 0; - this._ws.on('error', error => - this.emit('error', 'websocket', error.message, error)); const request = { command: 'subscribe', streams: ['ledger'] }; return this.request(request).then(data => { + if (_.isEmpty(data) || !data.ledger_index) { + // rippled instance doesn't have validated ledgers + return this._disconnect(false).then(() => { + throw new NotConnectedError('Rippled not initialized'); + }); + } + this._updateLedgerVersions(data); + + this._ws.removeListener('close', this._onUnexpectedCloseBound); + this._onUnexpectedCloseBound = + this._onUnexpectedClose.bind(this, false, null, null); + this._ws.once('close', this._onUnexpectedCloseBound); + + this._retry = 0; + this._ws.on('error', error => + this.emit('error', 'websocket', error.message, error)); + this._isReady = true; this.emit('connected'); + + return undefined; }); } @@ -252,8 +263,14 @@ class Connection extends EventEmitter { } disconnect() { - this._clearReconnectTimer(); - this._retry = 0; + return this._disconnect(true); + } + + _disconnect(calledByUser) { + if (calledByUser) { + this._clearReconnectTimer(); + this._retry = 0; + } return new Promise(resolve => { if (this._state === WebSocket.CLOSED) { resolve(); @@ -264,7 +281,9 @@ class Connection extends EventEmitter { this._ws.once('close', code => { this._ws = null; this._isReady = false; - this.emit('disconnected', code || 1000); // 1000 - CLOSE_NORMAL + if (calledByUser) { + this.emit('disconnected', code || 1000); // 1000 - CLOSE_NORMAL + } resolve(); }); this._ws.close(); diff --git a/src/common/serverinfo.js b/src/common/serverinfo.js index f1da5604..e6303ce0 100644 --- a/src/common/serverinfo.js +++ b/src/common/serverinfo.js @@ -1,4 +1,4 @@ -'use strict'; +'use strict'; // eslint-disable-line const _ = require('lodash'); const {convertKeysFromSnakeCaseToCamelCase} = require('./utils'); import type {Connection} from './connection'; @@ -43,18 +43,20 @@ function getServerInfo(connection: Connection): Promise { return connection.request({command: 'server_info'}).then(response => { const info = convertKeysFromSnakeCaseToCamelCase(response.info); renameKeys(info, {hostid: 'hostID'}); - renameKeys(info.validatedLedger, { - baseFeeXrp: 'baseFeeXRP', - reserveBaseXrp: 'reserveBaseXRP', - reserveIncXrp: 'reserveIncrementXRP', - seq: 'ledgerVersion' - }); - info.validatedLedger.baseFeeXRP = - info.validatedLedger.baseFeeXRP.toString(); - info.validatedLedger.reserveBaseXRP = - info.validatedLedger.reserveBaseXRP.toString(); - info.validatedLedger.reserveIncrementXRP = - info.validatedLedger.reserveIncrementXRP.toString(); + if (info.validatedLedger) { + renameKeys(info.validatedLedger, { + baseFeeXrp: 'baseFeeXRP', + reserveBaseXrp: 'reserveBaseXRP', + reserveIncXrp: 'reserveIncrementXRP', + seq: 'ledgerVersion' + }); + info.validatedLedger.baseFeeXRP = + info.validatedLedger.baseFeeXRP.toString(); + info.validatedLedger.reserveBaseXRP = + info.validatedLedger.reserveBaseXRP.toString(); + info.validatedLedger.reserveIncrementXRP = + info.validatedLedger.reserveIncrementXRP.toString(); + } return info; }); } diff --git a/test/api-test.js b/test/api-test.js index 0fc90328..2facd6d1 100644 --- a/test/api-test.js +++ b/test/api-test.js @@ -1,5 +1,5 @@ /* eslint-disable max-nested-callbacks */ -'use strict'; +'use strict'; // eslint-disable-line const _ = require('lodash'); const assert = require('assert-diff'); const setupAPI = require('./setup-api'); @@ -981,6 +981,19 @@ describe('RippleAPI', function() { }); }); + it('getServerInfo - no validated ledger', function() { + this.api.connection._send(JSON.stringify({ + command: 'config', + data: {serverInfoWithoutValidated: true} + })); + + return this.api.getServerInfo().then(info => { + assert.strictEqual(info.networkLedger, 'waiting'); + }).catch(error => { + assert(false, 'Should not throw Error, got ' + String(error)); + }); + }); + it('getFee', function() { return this.api.getFee().then(fee => { assert.strictEqual(fee, '0.000012'); diff --git a/test/connection-test.js b/test/connection-test.js index 1cb5c5f8..659f1373 100644 --- a/test/connection-test.js +++ b/test/connection-test.js @@ -379,4 +379,58 @@ describe('Connection', function() { }); this.api.connection._ws.emit('message', JSON.stringify(message)); }); + + it('should throw NotConnectedError if server does not have validated ledgers', + function() { + if (process.browser) { + // do not work in browser now, skipping + return false; + } + this.timeout(3000); + + this.api.connection._send(JSON.stringify({ + command: 'global_config', + data: {returnEmptySubscribeRequest: 1} + })); + + const api = new RippleAPI({server: this.api.connection._url}); + return api.connect().then(() => { + assert(false, 'Must have thrown!'); + }, error => { + assert(error instanceof this.api.errors.NotConnectedError, + 'Must throw NotConnectedError, got instead ' + String(error)); + }); + }); + + it('should try to reconnect on empty subscribe response on reconnect', + function(done) { + if (process.browser) { + // do not work in browser now, skipping + done(); + return; + } + this.timeout(23000); + + this.api.on('error', error => { + done(error || new Error('Should not emit error.')); + }); + let disconncedCount = 0; + this.api.on('connected', () => { + done(disconncedCount !== 1 ? + new Error('Wrong number of disconnects') : undefined); + }); + this.api.on('disconnected', () => { + disconncedCount++; + }); + + this.api.connection._send(JSON.stringify({ + command: 'global_config', + data: {returnEmptySubscribeRequest: 3} + })); + + this.api.connection._send(JSON.stringify({ + command: 'test_command', + data: {disconnectIn: 10} + })); + }); }); diff --git a/test/fixtures/rippled/empty.json b/test/fixtures/rippled/empty.json new file mode 100644 index 00000000..40e27a4e --- /dev/null +++ b/test/fixtures/rippled/empty.json @@ -0,0 +1,7 @@ +{ + "id": 0, + "status": "success", + "type": "response", + "result": { + } +} diff --git a/test/fixtures/rippled/index.js b/test/fixtures/rippled/index.js index 56191fc3..eec19ce3 100644 --- a/test/fixtures/rippled/index.js +++ b/test/fixtures/rippled/index.js @@ -1,4 +1,4 @@ -'use strict'; +'use strict'; // eslint-disable-line module.exports = { submit: { @@ -12,6 +12,7 @@ module.exports = { withSettingsTx: require('./ledger-with-settings-tx'), withStateAsHashes: require('./ledger-with-state-as-hashes') }, + empty: require('./empty'), subscribe: require('./subscribe'), unsubscribe: require('./unsubscribe'), account_info: { @@ -31,6 +32,7 @@ module.exports = { }, server_info: { normal: require('./server-info'), + noValidated: require('./server-info-no-validated'), error: require('./server-info-error') }, path_find: { diff --git a/test/fixtures/rippled/server-info-no-validated.json b/test/fixtures/rippled/server-info-no-validated.json new file mode 100644 index 00000000..636f5a1a --- /dev/null +++ b/test/fixtures/rippled/server-info-no-validated.json @@ -0,0 +1,31 @@ +{ + "id": 0, + "status": "success", + "type": "response", + "result": { + "info": { + "build_version": "0.30.1-hf2", + "complete_ledgers": "empty", + "hostid": "ARTS", + "io_latency_ms": 1, + "last_close": { + "converge_time_s": 2.007, + "proposers": 4 + }, + "load_factor": 1, + "peers": 53, + "pubkey_node": "n94wWvFUmaKGYrKUGgpv1DyYgDeXRGdACkNQaSe7zJiy5Znio7UC", + "server_state": "connected", + "network_ledger" : "waiting", + "closed_ledger": { + "age": 5, + "base_fee_xrp": 0.00001, + "hash": "4482DEE5362332F54A4036ED57EE1767C9F33CF7CE5A6670355C16CECE381D46", + "reserve_base_xrp": 20, + "reserve_inc_xrp": 5, + "seq": 6595042 + }, + "validation_quorum": 3 + } + } +} diff --git a/test/mock-rippled.js b/test/mock-rippled.js index 88a753ff..116277d2 100644 --- a/test/mock-rippled.js +++ b/test/mock-rippled.js @@ -1,4 +1,4 @@ -'use strict'; +'use strict'; // eslint-disable-line const _ = require('lodash'); const assert = require('assert'); const WebSocketServer = require('ws').Server; @@ -79,6 +79,8 @@ module.exports = function(port) { }); }); + mock.config = {}; + mock.onAny(function() { if (this.event.indexOf('request_') !== 0) { return; @@ -101,6 +103,18 @@ module.exports = function(port) { conn.config = _.assign(conn.config, request.data); }); + mock.on('request_test_command', function(request, conn) { + assert.strictEqual(request.command, 'test_command'); + if (request.data.disconnectIn) { + setTimeout(conn.terminate.bind(conn), request.data.disconnectIn); + } + }); + + mock.on('request_global_config', function(request, conn) { + assert.strictEqual(request.command, 'global_config'); + mock.config = _.assign(conn.config, request.data); + }); + mock.on('request_echo', function(request, conn) { assert.strictEqual(request.command, 'echo'); conn.send(JSON.stringify(request.data)); @@ -112,6 +126,8 @@ module.exports = function(port) { conn.send(createResponse(request, fixtures.server_info.error)); } else if (conn.config.disconnectOnServerInfo) { conn.close(); + } else if (conn.config.serverInfoWithoutValidated) { + conn.send(createResponse(request, fixtures.server_info.noValidated)); } else { conn.send(createResponse(request, fixtures.server_info.normal)); } @@ -119,7 +135,10 @@ module.exports = function(port) { mock.on('request_subscribe', function(request, conn) { assert.strictEqual(request.command, 'subscribe'); - if (request.accounts) { + if (mock.config.returnEmptySubscribeRequest) { + mock.config.returnEmptySubscribeRequest--; + conn.send(createResponse(request, fixtures.empty)); + } else if (request.accounts) { assert(_.indexOf(_.values(addresses), request.accounts[0]) !== -1); } conn.send(createResponse(request, fixtures.subscribe));