Skip to content

Commit

Permalink
BREAKING CHANGE: Change error event format and fix crash due to error…
Browse files Browse the repository at this point in the history
… event on websocket
  • Loading branch information
Chris Clark committed Dec 9, 2015
1 parent ad1d3e1 commit 9cd7259
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 16 deletions.
14 changes: 12 additions & 2 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -3499,12 +3499,22 @@ This event is emitted when there is an error on the connection to the server tha

### Return Value

The first parameter is a string indicating the error type, which may be `badMessage` (meaning that rippled returned a malformed message), or one of the [rippled Universal Errors](https://ripple.com/build/rippled-apis/#universal-errors). The second parameter is a message explaining the error, or the message that caused the error in the case of `badMessage`.
The first parameter is a string indicating the error type:
* `badMessage` - rippled returned a malformed message
* `websocket` - the websocket library emitted an error
* one of the error codes found in the [rippled Universal Errors](https://ripple.com/build/rippled-apis/#universal-errors).

The second parameter is a message explaining the error.

The third parameter is:
* the message that caused the error for `badMessage`
* the error object emitted for `websocket`
* the parsed response for rippled errors

### Example

```javascript
api.on('error', (errorCode, errorMessage) => {
api.on('error', (errorCode, errorMessage, data) => {
console.log(errorCode + ': ' + errorMessage);
});
```
Expand Down
14 changes: 12 additions & 2 deletions docs/src/events.md.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,22 @@ This event is emitted when there is an error on the connection to the server tha

### Return Value

The first parameter is a string indicating the error type, which may be `badMessage` (meaning that rippled returned a malformed message), or one of the [rippled Universal Errors](https://ripple.com/build/rippled-apis/#universal-errors). The second parameter is a message explaining the error, or the message that caused the error in the case of `badMessage`.
The first parameter is a string indicating the error type:
* `badMessage` - rippled returned a malformed message
* `websocket` - the websocket library emitted an error
* one of the error codes found in the [rippled Universal Errors](https://ripple.com/build/rippled-apis/#universal-errors).

The second parameter is a message explaining the error.

The third parameter is:
* the message that caused the error for `badMessage`
* the error object emitted for `websocket`
* the parsed response for rippled errors

### Example

```javascript
api.on('error', (errorCode, errorMessage) => {
api.on('error', (errorCode, errorMessage, data) => {
console.log(errorCode + ': ' + errorMessage);
});
```
Expand Down
4 changes: 2 additions & 2 deletions src/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ class RippleAPI extends EventEmitter {
this.connection.on('ledgerClosed', message => {
this.emit('ledger', server.formatLedgerClose(message));
});
this.connection.on('error', (type, info) => {
this.emit('error', type, info);
this.connection.on('error', (errorCode, errorMessage, data) => {
this.emit('error', errorCode, errorMessage, data);
});
} else {
// use null object pattern to provide better error message if user
Expand Down
3 changes: 2 additions & 1 deletion src/broadcast.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ class RippleAPIBroadcast extends RippleAPI {

apis.forEach(api => {
api.on('ledger', this.onLedgerEvent.bind(this));
api.on('error', (type, info) => this.emit('error', type, info));
api.on('error', (errorCode, errorMessage, data) =>
this.emit('error', errorCode, errorMessage, data));
});
}

Expand Down
10 changes: 8 additions & 2 deletions src/common/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class Connection extends EventEmitter {
}
return [data.type, data];
} else if (data.type === undefined && data.error) {
return ['error', data.error, data.error_message]; // e.g. slowDown
return ['error', data.error, data.error_message, data]; // e.g. slowDown
}
throw new ResponseFormatError('unrecognized message type: ' + data.type);
}
Expand All @@ -68,7 +68,7 @@ class Connection extends EventEmitter {
try {
parameters = this._parseMessage(message);
} catch (error) {
this.emit('error', 'badMessage', message);
this.emit('error', 'badMessage', error.message, message);
return;
}
// we don't want this inside the try/catch or exceptions in listener
Expand Down Expand Up @@ -161,6 +161,12 @@ class Connection extends EventEmitter {
this._ws.once('open', resolve);
} else {
this._ws = this._createWebSocket();
// when an error causes the connection to close, the close event
// 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.messsage, error));
this._ws.on('message', this._onMessage.bind(this));
this._onUnexpectedCloseBound = this._onUnexpectedClose.bind(this);
this._ws.once('close', this._onUnexpectedCloseBound);
Expand Down
17 changes: 10 additions & 7 deletions test/connection-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,9 @@ describe('Connection', function() {
});

it('invalid message id', function(done) {
this.api.on('error', (type, message) => {
assert.strictEqual(type, 'badMessage');
this.api.on('error', (errorCode, errorMessage, message) => {
assert.strictEqual(errorCode, 'badMessage');
assert.strictEqual(errorMessage, 'valid id not found in response');
assert.strictEqual(message,
'{"type":"response","id":"must be integer"}');
done();
Expand All @@ -239,9 +240,10 @@ describe('Connection', function() {
});

it('propagate error message', function(done) {
this.api.on('error', (type, message) => {
assert.strictEqual(type, 'slowDown');
assert.strictEqual(message, 'slow down');
this.api.on('error', (errorCode, errorMessage, data) => {
assert.strictEqual(errorCode, 'slowDown');
assert.strictEqual(errorMessage, 'slow down');
assert.deepEqual(data, {error: 'slowDown', error_message: 'slow down'});
done();
});
this.api.connection._onMessage(JSON.stringify({
Expand All @@ -250,8 +252,9 @@ describe('Connection', function() {
});

it('unrecognized message type', function(done) {
this.api.on('error', (type, message) => {
assert.strictEqual(type, 'badMessage');
this.api.on('error', (errorCode, errorMessage, message) => {
assert.strictEqual(errorCode, 'badMessage');
assert.strictEqual(errorMessage, 'unrecognized message type: unknown');
assert.strictEqual(message, '{"type":"unknown"}');
done();
});
Expand Down

0 comments on commit 9cd7259

Please sign in to comment.