From 6e9fcfc81d922a1b188268bf50d7e67c07d6d662 Mon Sep 17 00:00:00 2001 From: Enkot Date: Thu, 20 Dec 2018 17:02:28 +0200 Subject: [PATCH] feat(errors): sync/async error handling for lifecycle hooks and v-on handlers (#8395) close #6953, close #7653 --- src/core/instance/events.js | 6 +- src/core/instance/lifecycle.js | 6 +- src/core/util/error.js | 7 ++ src/core/vdom/helpers/update-listeners.js | 23 +++-- test/unit/features/error-handling.spec.js | 112 ++++++++++++++++++++++ 5 files changed, 144 insertions(+), 10 deletions(-) diff --git a/src/core/instance/events.js b/src/core/instance/events.js index bb07a96b4..fa2c8d4fd 100644 --- a/src/core/instance/events.js +++ b/src/core/instance/events.js @@ -5,7 +5,8 @@ import { toArray, hyphenate, handleError, - formatComponentName + formatComponentName, + handlePromiseError } from '../util/index' import { updateListeners } from '../vdom/helpers/index' @@ -135,7 +136,8 @@ export function eventsMixin (Vue: Class) { const args = toArray(arguments, 1) for (let i = 0, l = cbs.length; i < l; i++) { try { - cbs[i].apply(vm, args) + const cbResult = cbs[i].apply(vm, args) + handlePromiseError(cbResult, vm, `event handler for "${event}"`) } catch (e) { handleError(e, vm, `event handler for "${event}"`) } diff --git a/src/core/instance/lifecycle.js b/src/core/instance/lifecycle.js index d0b871be2..5efa0c614 100644 --- a/src/core/instance/lifecycle.js +++ b/src/core/instance/lifecycle.js @@ -15,7 +15,8 @@ import { remove, handleError, emptyObject, - validateProp + validateProp, + handlePromiseError } from '../util/index' export let activeInstance: any = null @@ -326,7 +327,8 @@ export function callHook (vm: Component, hook: string) { if (handlers) { for (let i = 0, j = handlers.length; i < j; i++) { try { - handlers[i].call(vm) + const fnResult = handlers[i].call(vm) + handlePromiseError(fnResult, vm, `${hook} hook`) } catch (e) { handleError(e, vm, `${hook} hook`) } diff --git a/src/core/util/error.js b/src/core/util/error.js index b7d05a6d7..5b5d60c80 100644 --- a/src/core/util/error.js +++ b/src/core/util/error.js @@ -24,6 +24,13 @@ export function handleError (err: Error, vm: any, info: string) { globalHandleError(err, vm, info) } +export function handlePromiseError (value: any, vm: any, info: string) { + // if value is promise, handle it (a promise must have a then function) + if (value && typeof value.then === 'function' && typeof value.catch === 'function') { + value.catch(e => handleError(e, vm, info)) + } +} + function globalHandleError (err, vm, info) { if (config.errorHandler) { try { diff --git a/src/core/vdom/helpers/update-listeners.js b/src/core/vdom/helpers/update-listeners.js index df58727f1..88cab4ec4 100644 --- a/src/core/vdom/helpers/update-listeners.js +++ b/src/core/vdom/helpers/update-listeners.js @@ -1,7 +1,6 @@ /* @flow */ -import { warn } from 'core/util/index' - +import { warn, handleError, handlePromiseError } from 'core/util/index' import { cached, isUndef, @@ -31,17 +30,29 @@ const normalizeEvent = cached((name: string): { } }) -export function createFnInvoker (fns: Function | Array): Function { +export function createFnInvoker (fns: Function | Array, vm: ?Component): Function { function invoker () { const fns = invoker.fns if (Array.isArray(fns)) { const cloned = fns.slice() for (let i = 0; i < cloned.length; i++) { - cloned[i].apply(null, arguments) + try { + const result = cloned[i].apply(null, arguments) + handlePromiseError(result, vm, 'v-on async') + } catch (e) { + handleError(e, vm, 'v-on') + } } } else { // return handler return value for single handlers - return fns.apply(null, arguments) + let result + try { + result = fns.apply(null, arguments) + handlePromiseError(result, vm, 'v-on async') + } catch (e) { + handleError(e, vm, 'v-on') + } + return result } } invoker.fns = fns @@ -73,7 +84,7 @@ export function updateListeners ( ) } else if (isUndef(old)) { if (isUndef(cur.fns)) { - cur = on[name] = createFnInvoker(cur) + cur = on[name] = createFnInvoker(cur, vm) } if (isTrue(event.once)) { cur = on[name] = createOnceHandler(event.name, cur, event.capture) diff --git a/test/unit/features/error-handling.spec.js b/test/unit/features/error-handling.spec.js index 713c91a76..2b10ea632 100644 --- a/test/unit/features/error-handling.spec.js +++ b/test/unit/features/error-handling.spec.js @@ -22,6 +22,23 @@ describe('Error handling', () => { }) }) + // hooks that can return rejected promise + ;[ + ['beforeCreate', 'beforeCreate hook'], + ['created', 'created hook'], + ['beforeMount', 'beforeMount hook'], + ['mounted', 'mounted hook'], + ['event', 'event handler for "e"'] + ].forEach(([type, description]) => { + it(`should recover from promise errors in ${type}`, done => { + createTestInstance(components[`${type}Async`]) + waitForUpdate(() => { + expect(`Error in ${description}`).toHaveBeenWarned() + expect(`Error: ${type}`).toHaveBeenWarned() + }).then(done) + }) + }) + // error in mounted hook should affect neither child nor parent it('should recover from errors in mounted hook', done => { const vm = createTestInstance(components.mounted) @@ -45,6 +62,20 @@ describe('Error handling', () => { }) }) + // hooks that can return rejected promise + ;[ + ['beforeUpdate', 'beforeUpdate hook'], + ['updated', 'updated hook'] + ].forEach(([type, description]) => { + it(`should recover from promise errors in ${type} hook`, done => { + const vm = createTestInstance(components[`${type}Async`]) + assertBothInstancesActive(vm).then(() => { + expect(`Error in ${description}`).toHaveBeenWarned() + expect(`Error: ${type}`).toHaveBeenWarned() + }).then(done) + }) + }) + ;[ ['beforeDestroy', 'beforeDestroy hook'], ['destroyed', 'destroyed hook'], @@ -62,6 +93,21 @@ describe('Error handling', () => { }) }) + ;[ + ['beforeDestroy', 'beforeDestroy hook'], + ['destroyed', 'destroyed hook'] + ].forEach(([type, description]) => { + it(`should recover from promise errors in ${type} hook`, done => { + const vm = createTestInstance(components[`${type}Async`]) + vm.ok = false + setTimeout(() => { + expect(`Error in ${description}`).toHaveBeenWarned() + expect(`Error: ${type}`).toHaveBeenWarned() + assertRootInstanceActive(vm).then(done) + }) + }) + }) + it('should recover from errors in user watcher getter', done => { const vm = createTestInstance(components.userWatcherGetter) vm.n++ @@ -152,6 +198,40 @@ describe('Error handling', () => { expect(vm.$el.textContent).toContain('error in render') Vue.config.errorHandler = null }) + + // event handlers that can throw errors or return rejected promise + ;[ + ['single handler', '
'], + ['multiple handlers', '
'] + ].forEach(([type, template]) => { + it(`should recover from v-on errors for ${type} registered`, () => { + const vm = new Vue({ + template, + methods: { bork () { throw new Error('v-on') } } + }).$mount() + document.body.appendChild(vm.$el) + triggerEvent(vm.$el, 'click') + expect('Error in v-on').toHaveBeenWarned() + expect('Error: v-on').toHaveBeenWarned() + document.body.removeChild(vm.$el) + }) + + it(`should recover from v-on async errors for ${type} registered`, (done) => { + const vm = new Vue({ + template, + methods: { bork () { + return new Promise((resolve, reject) => reject(new Error('v-on async'))) + } } + }).$mount() + document.body.appendChild(vm.$el) + triggerEvent(vm.$el, 'click') + waitForUpdate(() => { + expect('Error in v-on async').toHaveBeenWarned() + expect('Error: v-on async').toHaveBeenWarned() + document.body.removeChild(vm.$el) + }).then(done) + }) + }) }) function createErrorTestComponents () { @@ -188,6 +268,16 @@ function createErrorTestComponents () { throw new Error(before) } + const beforeCompAsync = components[`${before}Async`] = { + props: ['n'], + render (h) { + return h('div', this.n) + } + } + beforeCompAsync[before] = function () { + return new Promise((resolve, reject) => reject(new Error(before))) + } + // after const after = hook.replace(/e?$/, 'ed') const afterComp = components[after] = { @@ -199,6 +289,16 @@ function createErrorTestComponents () { afterComp[after] = function () { throw new Error(after) } + + const afterCompAsync = components[`${after}Async`] = { + props: ['n'], + render (h) { + return h('div', this.n) + } + } + afterCompAsync[after] = function () { + return new Promise((resolve, reject) => reject(new Error(after))) + } }) // directive hooks errors @@ -272,6 +372,18 @@ function createErrorTestComponents () { } } + components.eventAsync = { + beforeCreate () { + this.$on('e', () => new Promise((resolve, reject) => reject(new Error('event')))) + }, + mounted () { + this.$emit('e') + }, + render (h) { + return h('div') + } + } + return components }