Skip to content

Commit

Permalink
FABN-1050: Catch errors thrown by event callbacks
Browse files Browse the repository at this point in the history
Catch (and log) any errors thrown by transaction and block
event listener onEvent and onError callbacks.

Change-Id: Ic0607b86b89cc7150ca372b1f14cac8652ccdc58
Signed-off-by: Mark S. Lewis <mark_lewis@uk.ibm.com>
  • Loading branch information
bestbeforetoday committed Dec 5, 2018
1 parent cfddbb5 commit 003da0d
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 6 deletions.
20 changes: 18 additions & 2 deletions fabric-client/lib/ChannelEventHub.js
Original file line number Diff line number Diff line change
Expand Up @@ -1533,8 +1533,8 @@ class EventRegistration {
* setting if not option setting is set by the user
*/
constructor(onEvent, onError, options, default_unregister, default_disconnect) {
this.onEvent = onEvent;
this.onError = onError;
this._onEventFn = onEvent;
this._onErrorFn = onError;
this.unregister = default_unregister;
this.disconnect = default_disconnect;
this.unregister_action = () => { }; // do nothing by default
Expand All @@ -1556,4 +1556,20 @@ class EventRegistration {
}
}
}

onEvent(...args) {
try {
this._onEventFn(...args);
} catch (error) {
logger.warn('Event notification callback failed', error);
}
}

onError(...args) {
try {
this._onErrorFn(...args);
} catch (error) {
logger.warn('Error notifacation callback failed', error);
}
}
}
67 changes: 63 additions & 4 deletions fabric-client/test/ChannelEventHub.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,20 +487,48 @@ describe('ChannelEventHub', () => {
});

describe('#disconnect', () => {
let hub;

beforeEach(() => {
hub = new ChannelEventHub('channel', 'peer');
});

it('should call a debug log', () => {
const hub = new ChannelEventHub('channel', 'peer');
hub._disconnect_running = true;
hub.disconnect();
sinon.assert.calledWith(FakeLogger.debug, 'disconnect - disconnect is running');
});

it('should log, call _disconnect and change disconnect_running to false', () => {
const hub = new ChannelEventHub('channel', 'peer');
hub._disconnect = sandbox.stub();
hub.disconnect();
hub._disconnect_running.should.be.false;
sinon.assert.calledWith(hub._disconnect, sinon.match(Error));
});

it('handles errors thrown by block event listeners', () => {
const onEventStub = sandbox.stub().throws('onEvent');
const onErrorStub = sandbox.stub().throws('onError');
hub.registerBlockEvent(onEventStub, onErrorStub, {});
hub.registerBlockEvent(onEventStub, onErrorStub, {});

hub.disconnect();

sinon.assert.calledTwice(onErrorStub);
});

it('handles errors throw by tx event listeners', () => {
const onEventStub = sandbox.stub().throws('onEvent');
const txErrorStub = sandbox.stub().throws('tx onError');
const allErrorStub = sandbox.stub().throws('all onError');
hub.registerTxEvent('1', onEventStub, txErrorStub, {});
hub.registerTxEvent('all', onEventStub, allErrorStub, {});

hub.disconnect();

sinon.assert.calledOnce(txErrorStub);
sinon.assert.calledOnce(allErrorStub);
});
});

describe('#close', () => {
Expand Down Expand Up @@ -1528,6 +1556,17 @@ describe('ChannelEventHub', () => {
sinon.assert.calledWith(FakeLogger.debug, '_processBlockEvents - calling block listener callback');
sinon.assert.calledWith(onEventStub, 'block');
});

it('handles errors thrown from block listeners', () => {
const onEventStub = sandbox.stub().throws('onEvent');
const onErrorStub = sandbox.stub().throws('onError');
hub.registerBlockEvent(onEventStub, onErrorStub, {});
hub.registerBlockEvent(onEventStub, onErrorStub, {});

hub._processBlockEvents('block');

sinon.assert.calledTwice(onEventStub);
});
});

describe('#_processTxEvents', () => {
Expand Down Expand Up @@ -1576,6 +1615,26 @@ describe('ChannelEventHub', () => {
sinon.assert.calledWith(_checkTransactionIdStub, 1, 'code0', 1);
sinon.assert.calledWith(_checkTransactionIdStub, 1, 'code1', 1);
});

it('handles errors thrown from tx event listeners', () => {
hub = new ChannelEventHub('channel', 'peer');
const fakeTx1 = {txid: '1'};
const fakeTx2 = {txid: '2'};
const block = {
number: 1,
filtered_transactions: [fakeTx1, fakeTx2]
};
const txOnEventStub = sandbox.stub().throws('tx onEvent');
const allOnEventStub = sandbox.stub().throws('all onEvent');
const onErrorStub = sandbox.stub().throws('onError');
hub.registerTxEvent(fakeTx1.txid, txOnEventStub, onErrorStub, {});
hub.registerTxEvent('all', allOnEventStub, onErrorStub, {});

hub._processTxEvents(block);

sinon.assert.calledOnce(txOnEventStub);
sinon.assert.calledTwice(allOnEventStub);
});
});

describe('#_checkTransactionId', () => {
Expand Down Expand Up @@ -2048,8 +2107,8 @@ describe('EventRegistration', () => {

it('should set the correct parameters', () => {
const reg = new EventRegistration('onEvent', 'onError', null, 'default_unregister', 'default_disconnect');
reg.onEvent.should.equal('onEvent');
reg.onError.should.equal('onError');
reg._onEventFn.should.equal('onEvent');
reg._onErrorFn.should.equal('onError');
reg.unregister.should.equal('default_unregister');
reg.disconnect.should.equal('default_disconnect');
reg.unregister_action.should.be.instanceof(Function);
Expand Down

0 comments on commit 003da0d

Please sign in to comment.