From 54f12862dc0000a47e73ad6011b25958674a513d Mon Sep 17 00:00:00 2001 From: Elliot Lee Date: Sat, 12 May 2018 09:38:49 -0700 Subject: [PATCH] Improve errors (#893) - `RippledError`: Include the full response from the `rippled` server. - A new test ensures correct behavior when `streams` is not an array. - `NotConnectedError` may be thrown with a different message than before. --- HISTORY.md | 6 ++++++ src/common/connection.ts | 10 ++++----- test/connection-test.js | 25 ++++++++++++++++------ test/fixtures/rippled/index.js | 1 + test/fixtures/rippled/subscribe_error.json | 13 +++++++++++ test/mock-rippled.js | 4 +++- 6 files changed, 47 insertions(+), 12 deletions(-) create mode 100644 test/fixtures/rippled/subscribe_error.json diff --git a/HISTORY.md b/HISTORY.md index 9e829008..7ee698ab 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,11 @@ # ripple-lib Release History +## 1.0.0 (UNRELEASED) + ++ Potentially breaking change: Improve errors. For example, `RippledError` now includes the full response from + the `rippled` server ([#687](https://github.com/ripple/ripple-lib/issues/687)). `NotConnectedError` + may be thrown with a different message than before. + ## 1.0.0-beta.0 (2018-05-10) + [Add `request`, `hasNextPage`, and diff --git a/src/common/connection.ts b/src/common/connection.ts index f9a6daf0..1f4f58fc 100644 --- a/src/common/connection.ts +++ b/src/common/connection.ts @@ -84,7 +84,7 @@ class Connection extends EventEmitter { const data = JSON.parse(message) if (data.type === 'response') { if (!(Number.isInteger(data.id) && data.id >= 0)) { - throw new ResponseFormatError('valid id not found in response') + throw new ResponseFormatError('valid id not found in response', data) } return [data.id.toString(), data] } else if (data.type === undefined && data.error) { @@ -241,7 +241,7 @@ class Connection extends EventEmitter { _onOpenError(reject, error) { this._onOpenErrorBound = null this._unbindOnUnxpectedClose() - reject(new NotConnectedError(error && error.message)) + reject(new NotConnectedError(error.message, error)) } _createWebSocket(): WebSocket { @@ -399,7 +399,7 @@ class Connection extends EventEmitter { return new Promise((resolve, reject) => { this._ws.send(message, undefined, error => { if (error) { - reject(new DisconnectedError(error.message)) + reject(new DisconnectedError(error.message, error)) } else { resolve() } @@ -445,12 +445,12 @@ class Connection extends EventEmitter { this.once(eventName, response => { if (response.status === 'error') { - _reject(new RippledError(response.error)) + _reject(new RippledError(response.error, response)) } else if (response.status === 'success') { _resolve(response.result) } else { _reject(new ResponseFormatError( - 'unrecognized status: ' + response.status)) + 'unrecognized status: ' + response.status, response)) } }) diff --git a/test/connection-test.js b/test/connection-test.js index a90678b0..cbc7b211 100644 --- a/test/connection-test.js +++ b/test/connection-test.js @@ -381,7 +381,7 @@ describe('Connection', function() { })); }); - it('propagate error message', function(done) { + it('propagates error message', function(done) { this.api.on('error', (errorCode, errorMessage, data) => { assert.strictEqual(errorCode, 'slowDown'); assert.strictEqual(errorMessage, 'slow down'); @@ -393,6 +393,19 @@ describe('Connection', function() { })); }); + it('propagates RippledError data', function(done) { + this.api.request('subscribe', {streams: 'validations'}).catch(error => { + assert.strictEqual(error.name, 'RippledError') + assert.strictEqual(error.message, 'invalidParams') + assert.strictEqual(error.data.error_code, 31) + assert.strictEqual(error.data.error_message, 'Invalid parameters.') + assert.deepEqual(error.data.request, { command: 'subscribe', id: 0, streams: 'validations' }) + assert.strictEqual(error.data.status, 'error') + assert.strictEqual(error.data.type, 'response') + done() + }) + }); + it('unrecognized message type', function(done) { // This enables us to automatically support any // new messages added by rippled in the future. @@ -414,8 +427,8 @@ describe('Connection', function() { }); it('should throw RippledNotInitializedError if server does not have ' + - 'validated ledgers', - function() { + 'validated ledgers', function() { + this.timeout(3000); this.api.connection._send(JSON.stringify({ @@ -439,13 +452,13 @@ describe('Connection', function() { this.api.on('error', error => { done(error || new Error('Should not emit error.')); }); - let disconncedCount = 0; + let disconnectedCount = 0; this.api.on('connected', () => { - done(disconncedCount !== 1 ? + done(disconnectedCount !== 1 ? new Error('Wrong number of disconnects') : undefined); }); this.api.on('disconnected', () => { - disconncedCount++; + disconnectedCount++; }); this.api.connection._send(JSON.stringify({ diff --git a/test/fixtures/rippled/index.js b/test/fixtures/rippled/index.js index 014bceb0..fe148c6d 100644 --- a/test/fixtures/rippled/index.js +++ b/test/fixtures/rippled/index.js @@ -16,6 +16,7 @@ module.exports = { }, empty: require('./empty'), subscribe: require('./subscribe'), + subscribe_error: require('./subscribe_error'), unsubscribe: require('./unsubscribe'), account_objects: { normal: require('./account-objects'), diff --git a/test/fixtures/rippled/subscribe_error.json b/test/fixtures/rippled/subscribe_error.json new file mode 100644 index 00000000..dda61473 --- /dev/null +++ b/test/fixtures/rippled/subscribe_error.json @@ -0,0 +1,13 @@ +{ + "id": 0, + "status": "error", + "type": "response", + "error": "invalidParams", + "error_code": 31, + "error_message": "Invalid parameters.", + "request": { + "command": "subscribe", + "id": 0, + "streams": "validations" + } +} diff --git a/test/mock-rippled.js b/test/mock-rippled.js index 1b0a0bbd..39bdbb3c 100644 --- a/test/mock-rippled.js +++ b/test/mock-rippled.js @@ -168,7 +168,9 @@ module.exports = function createMockRippled(port) { mock.on('request_subscribe', function (request, conn) { assert.strictEqual(request.command, 'subscribe'); - if (mock.config.returnEmptySubscribeRequest) { + if (request && request.streams === 'validations') { + conn.send(createResponse(request, fixtures.subscribe_error)) + } else if (mock.config.returnEmptySubscribeRequest) { mock.config.returnEmptySubscribeRequest--; conn.send(createResponse(request, fixtures.empty)); } else if (request.accounts) {