Wrap API event handlers in try/catch blocks
This prevents one bad handler from stopping others in the queue
from being executed.
Change-Id: Id1702d0181f8c80af0b41703f3e5871f08a6e0a6
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js
index 147ef9f..862a2d2 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js
@@ -59,9 +59,13 @@
canSubmitChange: function() {
var submitCallbacks = this._getEventCallbacks(EventType.SUBMIT_CHANGE);
-
var cancelSubmit = submitCallbacks.some(function(callback) {
- return callback() === false;
+ try {
+ return callback() === false;
+ } catch (err) {
+ console.error(err);
+ }
+ return false;
});
return !cancelSubmit;
@@ -75,7 +79,11 @@
_handleHistory: function(detail) {
this._getEventCallbacks(EventType.HISTORY).forEach(function(cb) {
- cb(detail.path);
+ try {
+ cb(detail.path);
+ } catch (err) {
+ console.error(err);
+ }
});
},
@@ -90,13 +98,21 @@
break;
}
}
- cb(change, revision);
+ try {
+ cb(change, revision);
+ } catch (err) {
+ console.error(err);
+ }
});
},
_handleComment: function(detail) {
this._getEventCallbacks(EventType.COMMENT).forEach(function(cb) {
- cb(detail.node);
+ try {
+ cb(detail.node);
+ } catch (err) {
+ console.error(err);
+ }
});
},
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
index c10c73c..3be232e 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
@@ -32,9 +32,14 @@
suite('gr-js-api-interface tests', function() {
var element;
var plugin;
+ var errorStub;
+ var throwErrFn = function() {
+ throw Error('Unfortunately, this handler has stopped');
+ };
setup(function() {
element = fixture('basic');
+ errorStub = sinon.stub(console, 'error');
Gerrit.install(function(p) { plugin = p; },
'http://test.com/plugins/testplugin/static/test.js');
});
@@ -42,6 +47,7 @@
teardown(function() {
element._removeEventCallbacks();
plugin = null;
+ errorStub.restore();
});
test('url', function() {
@@ -51,8 +57,10 @@
});
test('history event', function(done) {
+ plugin.on(element.EventType.HISTORY, throwErrFn);
plugin.on(element.EventType.HISTORY, function(path) {
assert.equal(path, '/path/to/awesomesauce');
+ assert.isTrue(errorStub.calledOnce);
done();
});
element.handleEvent(element.EventType.HISTORY,
@@ -67,9 +75,11 @@
abc: {_number: 1},
},
};
+ plugin.on(element.EventType.SHOW_CHANGE, throwErrFn);
plugin.on(element.EventType.SHOW_CHANGE, function(change, revision) {
assert.deepEqual(change, testChange);
assert.deepEqual(revision, testChange.revisions.abc);
+ assert.isTrue(errorStub.calledOnce);
done();
});
element.handleEvent(element.EventType.SHOW_CHANGE,
@@ -78,19 +88,24 @@
test('comment event', function(done) {
var testCommentNode = {foo: 'bar'};
+ plugin.on(element.EventType.COMMENT, throwErrFn);
plugin.on(element.EventType.COMMENT, function(commentNode) {
assert.deepEqual(commentNode, testCommentNode);
+ assert.isTrue(errorStub.calledOnce);
done();
});
element.handleEvent(element.EventType.COMMENT, {node: testCommentNode});
});
test('submitchange', function() {
+ plugin.on(element.EventType.SUBMIT_CHANGE, throwErrFn);
plugin.on(element.EventType.SUBMIT_CHANGE, function() { return true; });
assert.isTrue(element.canSubmitChange());
+ assert.isTrue(errorStub.calledOnce);
plugin.on(element.EventType.SUBMIT_CHANGE, function() { return false; });
plugin.on(element.EventType.SUBMIT_CHANGE, function() { return true; });
assert.isFalse(element.canSubmitChange());
+ assert.isTrue(errorStub.calledTwice);
});
});
