From e0a18d3394a61d059f743612052b6a2a809f138d Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Mon, 8 Jul 2019 19:09:53 -0700 Subject: [PATCH] blueslip: Replace jQuery wrappers with error event listener. Not all our errors actually happen in the contexts we were wrapping (e.g. `setTimeout` and `_.throttle`). Also this fixes the neat Firefox inspector feature that shows you where your event handlers for a given DOM element actually live. Using this "semi-modern" browser event means that Safari 9 and older and IE10 and older may not have our browser error reporting active; that seems fine giving the vanishing market share of those browsers. https://blog.sentry.io/2016/01/04/client-javascript-reporting-window-onerror Signed-off-by: Anders Kaseorg --- frontend_tests/node_tests/zblueslip.js | 8 -- frontend_tests/zjsunit/zblueslip.js | 6 -- static/js/blueslip.js | 144 ++----------------------- 3 files changed, 7 insertions(+), 151 deletions(-) diff --git a/frontend_tests/node_tests/zblueslip.js b/frontend_tests/node_tests/zblueslip.js index b0f6c3c74b..7b6af0d1c3 100644 --- a/frontend_tests/node_tests/zblueslip.js +++ b/frontend_tests/node_tests/zblueslip.js @@ -85,12 +85,4 @@ run_test('basics', () => { throw_a_warning(); assert.equal(blueslip.get_test_logs('warn').length, 1); blueslip.clear_test_data(); - - // Finally, let's check the wrap_function feature which allows logging - // of errors in function calls. We can use it as follows: - const original_function = () => { - return 'hello'; - }; - const wrapped_function = blueslip.wrap_function(original_function); - assert.equal(original_function(), wrapped_function()); }); diff --git a/frontend_tests/zjsunit/zblueslip.js b/frontend_tests/zjsunit/zblueslip.js index bf81575679..7acdebddb6 100644 --- a/frontend_tests/zjsunit/zblueslip.js +++ b/frontend_tests/zjsunit/zblueslip.js @@ -81,12 +81,6 @@ exports.make_zblueslip = function (opts) { return ex.message; }; - lib.wrap_function = (f) => { - return (...args) => { - return f.apply(this, args); - }; - }; - return lib; }; diff --git a/static/js/blueslip.js b/static/js/blueslip.js index 87c7f635de..560b7043e4 100644 --- a/static/js/blueslip.js +++ b/static/js/blueslip.js @@ -217,144 +217,14 @@ exports.exception_msg = function blueslip_exception_msg(ex) { return message; }; -exports.wrap_function = function blueslip_wrap_function(func) { - if (func.blueslip_wrapper !== undefined) { - func.blueslip_wrapper_refcnt++; - return func.blueslip_wrapper; +$(window).on('error', function (event) { + var ex = event.originalEvent.error; + if (ex instanceof BlueslipError) { + return; } - var new_func = function blueslip_wrapper() { - try { - return func.apply(this, arguments); - } catch (ex) { - // Treat exceptions like a call to fatal() if they - // weren't generated from fatal() - if (ex instanceof BlueslipError) { - throw ex; - } - - var message = exports.exception_msg(ex); - report_error(message, ex.stack); - throw ex; - } - }; - func.blueslip_wrapper = new_func; - func.blueslip_wrapper_refcnt = 1; - return new_func; -}; - -// Catch all exceptions from jQuery event handlers, $(document).ready -// functions, and ajax success/failure continuations and funnel them -// through blueslip. -(function () { - // This reference counting scheme can't break all the circular - // references we create because users can remove jQuery event - // handlers without referencing the particular handler they want - // to remove. We just hope this memory leak won't be too bad. - function dec_wrapper_refcnt(func) { - if (func.blueslip_wrapper_refcnt !== undefined) { - func.blueslip_wrapper_refcnt--; - if (func.blueslip_wrapper_refcnt === 0) { - delete func.blueslip_wrapper; - delete func.blueslip_wrapper_refcnt; - } - } - } - - $.ajaxPrefilter(function (options) { - _.each(['success', 'error', 'complete'], function (cb_name) { - if (options[cb_name] !== undefined) { - options[cb_name] = exports.wrap_function(options[cb_name]); - } - }); - }); - - if (document.addEventListener) { - var orig_on = $.fn.on; - var orig_off = $.fn.off; - var orig_ready = $.fn.ready; - - $.fn.on = function blueslip_jquery_on_wrapper(types, selector, data, fn, one) { - if (typeof types === 'object') { - // ( types-Object, selector, data) - // We'll get called again from the recursive call in the original - // $.fn.on - return orig_on.call(this, types, selector, data, fn, one); - } - - // Only one handler, but we have to figure out which - // argument it is. The argument munging is taken from - // jQuery itself, so we tell jslint to ignore the style - // issues that the jQuery code would raise. It sucks - // that we have to replicate the code :( - /*jslint eqeq: true */ - if ( data == null && fn == null ) { - // ( types, fn ) - fn = selector; - data = selector = undefined; - } else if ( fn == null ) { - if ( typeof selector === "string" ) { - // ( types, selector, fn ) - fn = data; - data = undefined; - } else { - // ( types, data, fn ) - fn = data; - data = selector; - selector = undefined; - } - } - if ( fn === false ) { - fn = function () { return false; }; - } else if ( !fn ) { - return this; - } - /*jslint eqeq: false */ - - return orig_on.call(this, types, selector, data, exports.wrap_function(fn), one); - }; - - $.fn.off = function (types, selector, fn) { - if (types && types.preventDefault && types.handleObj) { - // (event) - // We'll get called again through the recursive call in the original - // $.fn.off - return orig_off.call(this, types, selector, fn); - } - - if (typeof types === "object" ) { - // ( types-object [, selector] ) - // We'll get called again through the recursive call in the original - // $.fn.off - return orig_off.call(this, types, selector, fn); - } - - // Only one handler, but we have to figure out which - // argument it is. The argument munging is taken from - // jQuery, itself. - if ( selector === false || typeof selector === "function" ) { - // ( types [, fn] ) - fn = selector; - selector = undefined; - } - if ( fn === false ) { - fn = function () { return false; }; - } - - if (fn) { - var wrapper = fn.blueslip_wrapper; - if (wrapper !== undefined) { - dec_wrapper_refcnt(fn); - fn = wrapper; - } - } - return orig_off.call(this, types, selector, fn); - }; - - $.fn.ready = function blueslip_jquery_ready_wrapper(func) { - return orig_ready.call(this, exports.wrap_function(func)); - }; - } -}()); + var message = exports.exception_msg(ex); + report_error(message, ex.stack); +}); function build_arg_list(msg, more_info) { var args = [msg];