From cc2c3edaa6dfd155a3c8747e81119b811a60b410 Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Wed, 19 Jul 2017 11:59:08 -0700 Subject: [PATCH] Override Backbone's trigger with one that catches exceptions Model operations are vulnerable to exceptions thrown by event handlers. Because this can interrupt really important data operations, it's better to let the operation continue and log the error. In all likelihood it's a view-related problem, and that shouldn't cause any data operation to fail. FREEBIE --- background.html | 1 + js/reliable_trigger.js | 76 +++++++++++++++--- test/index.html | 2 + test/reliable_trigger_test.js | 147 ++++++++++++++++++++++++++++++++++ 4 files changed, 217 insertions(+), 9 deletions(-) create mode 100644 test/reliable_trigger_test.js diff --git a/background.html b/background.html index 64db864a9..3e224f7f4 100644 --- a/background.html +++ b/background.html @@ -651,6 +651,7 @@ + diff --git a/js/reliable_trigger.js b/js/reliable_trigger.js index b5b961fcf..1b8cd06e0 100644 --- a/js/reliable_trigger.js +++ b/js/reliable_trigger.js @@ -1,5 +1,14 @@ (function () { - // Pure copy from Backbone. + // Note: this is all the code required to customize Backbone's trigger() method to make + // it resilient to exceptions thrown by event handlers. Indentation and code styles + // were kept inline with the Backbone implementation for easier diffs. + + // The changes are: + // 1. added 'name' parameter to triggerEvents to give it access to the current event name + // 2. added try/catch handlers to triggerEvents with error logging inside every while loop + + // And of course, we update the protoypes of Backbone.Model/Backbone.View as well as + // Backbone.Events itself // jscs:disable @@ -39,14 +48,62 @@ // A difficult-to-believe, but optimized internal dispatch function for // triggering events. Tries to keep the usual cases speedy (most internal // Backbone events have 3 arguments). - var triggerEvents = function(events, args) { + var triggerEvents = function(events, name, args) { var ev, i = -1, l = events.length, a1 = args[0], a2 = args[1], a3 = args[2]; + var logError = function(error) { + console.log('Model caught error triggering', name, 'event:', error && error.stack ? error.stack : error); + }; switch (args.length) { - case 0: while (++i < l) (ev = events[i]).callback.call(ev.ctx); return; - case 1: while (++i < l) (ev = events[i]).callback.call(ev.ctx, a1); return; - case 2: while (++i < l) (ev = events[i]).callback.call(ev.ctx, a1, a2); return; - case 3: while (++i < l) (ev = events[i]).callback.call(ev.ctx, a1, a2, a3); return; - default: while (++i < l) (ev = events[i]).callback.apply(ev.ctx, args); return; + case 0: + while (++i < l) { + try { + (ev = events[i]).callback.call(ev.ctx); + } + catch (error) { + logError(error); + } + } + return; + case 1: + while (++i < l) { + try { + (ev = events[i]).callback.call(ev.ctx, a1); + } + catch (error) { + logError(error); + } + } + return; + case 2: + while (++i < l) { + try { + (ev = events[i]).callback.call(ev.ctx, a1, a2); + } + catch (error) { + logError(error); + } + } + return; + case 3: + while (++i < l) { + try { + (ev = events[i]).callback.call(ev.ctx, a1, a2, a3); + } + catch (error) { + logError(error); + } + } + return; + default: + while (++i < l) { + try { + (ev = events[i]).callback.apply(ev.ctx, args); + } + catch (error) { + logError(error); + } + } + return; } }; @@ -60,8 +117,9 @@ if (!eventsApi(this, 'trigger', name, args)) return this; var events = this._events[name]; var allEvents = this._events.all; - if (events) triggerEvents(events, args); - if (allEvents) triggerEvents(allEvents, arguments); + if (events) triggerEvents(events, name, args); + if (allEvents) triggerEvents(allEvents, name, arguments); return this; }; })(); + diff --git a/test/index.html b/test/index.html index d5d2bf808..f138b1023 100644 --- a/test/index.html +++ b/test/index.html @@ -550,6 +550,7 @@ + @@ -624,6 +625,7 @@ + diff --git a/test/reliable_trigger_test.js b/test/reliable_trigger_test.js new file mode 100644 index 000000000..56f84351d --- /dev/null +++ b/test/reliable_trigger_test.js @@ -0,0 +1,147 @@ +'use strict'; + +describe('ReliableTrigger', function() { + describe('trigger', function() { + var Model, model; + + before(function() { + Model = Backbone.Model; + }); + + beforeEach(function() { + model = new Model(); + }); + + it('returns successfully if this._events is falsey', function() { + model._events = null; + model.trigger('click'); + }); + it('handles map of events to trigger', function() { + var a = 0, b = 0; + model.on('a', function(arg) { + a = arg; + }); + model.on('b', function(arg) { + b = arg; + }); + + model.trigger({ + a: 1, + b: 2 + }); + + assert.strictEqual(a, 1); + assert.strictEqual(b, 2); + }); + it('handles space-separated list of events to trigger', function() { + var a = false, b = false; + model.on('a', function() { + a = true; + }); + model.on('b', function() { + b = true; + }); + + model.trigger('a b'); + + assert.strictEqual(a, true); + assert.strictEqual(b, true); + }); + it('calls all clients registered for "all" event', function() { + var count = 0; + model.on('all', function() { + count += 1; + }); + + model.trigger('left'); + model.trigger('right'); + + assert.strictEqual(count, 2); + }); + it('calls all clients registered for target event', function() { + var a = false, b = false; + model.on('event', function() { + a = true; + }); + model.on('event', function() { + b = true; + }); + + model.trigger('event'); + + assert.strictEqual(a, true); + assert.strictEqual(b, true); + }); + it('successfully returns and calls all clients even if first failed', function() { + var a = false, b = false; + model.on('event', function() { + a = true; + throw new Error('a is set, but exception is thrown'); + }); + model.on('event', function() { + b = true; + }); + + model.trigger('event'); + + assert.strictEqual(a, true); + assert.strictEqual(b, true); + }); + it('calls clients with no args', function() { + var called = false; + model.on('event', function() { + called = true; + }); + + model.trigger('event'); + + assert.strictEqual(called, true); + }); + it('calls clients with 1 arg', function() { + var args; + model.on('event', function() { + args = arguments; + }); + + model.trigger('event', 1); + + assert.strictEqual(args[0], 1); + }); + it('calls clients with 2 args', function() { + var args; + model.on('event', function() { + args = arguments; + }); + + model.trigger('event', 1, 2); + + assert.strictEqual(args[0], 1); + assert.strictEqual(args[1], 2); + }); + it('calls clients with 3 args', function() { + var args; + model.on('event', function() { + args = arguments; + }); + + model.trigger('event', 1, 2, 3); + + assert.strictEqual(args[0], 1); + assert.strictEqual(args[1], 2); + assert.strictEqual(args[2], 3); + }); + it('calls clients with 4+ args', function() { + var args; + model.on('event', function() { + args = arguments; + }); + + model.trigger('event', 1, 2, 3, 4); + + assert.strictEqual(args[0], 1); + assert.strictEqual(args[1], 2); + assert.strictEqual(args[2], 3); + assert.strictEqual(args[3], 4); + }); + }); +});