improve error handling for lifecycle hooks

Author: yyx990803

Diff for: src/core/instance/lifecycle.js

@@ -6,7 +6,7 @@ import { createEmptyVNode } from '../vdom/vnode'
 import { observerState } from '../observer/index'
 import { updateComponentListeners } from './events'
 import { resolveSlots } from './render-helpers/resolve-slots'
-import { warn, validateProp, remove, noop, emptyObject } from '../util/index'
+import { warn, validateProp, remove, noop, emptyObject, handleError } from '../util/index'
 
 export let activeInstance: any = null
 
@@ -262,7 +262,11 @@ export function callHook (vm: Component, hook: string) {
   const handlers = vm.$options[hook]
   if (handlers) {
     for (let i = 0, j = handlers.length; i < j; i++) {
-      handlers[i].call(vm)
+      try {
+        handlers[i].call(vm)
+      } catch (e) {
+        handleError(e, vm, `${hook} hook`)
+      }
     }
   }
   if (vm._hasHookEvent) {

Diff for: src/core/instance/render.js

@@ -1,16 +1,14 @@
 /* @flow */
 
-import config from '../config'
-
 import {
   warn,
   nextTick,
   toNumber,
   _toString,
   looseEqual,
   emptyObject,
-  looseIndexOf,
-  formatComponentName
+  handleError,
+  looseIndexOf
 } from '../util/index'
 
 import VNode, {
@@ -79,15 +77,7 @@ export function renderMixin (Vue: Class<Component>) {
     try {
       vnode = render.call(vm._renderProxy, vm.$createElement)
     } catch (e) {
-      /* istanbul ignore else */
-      if (config.errorHandler) {
-        config.errorHandler.call(null, e, vm)
-      } else {
-        if (process.env.NODE_ENV !== 'production') {
-          warn(`Error when rendering ${formatComponentName(vm)}:`)
-        }
-        throw e
-      }
+      handleError(e, vm, `render function`)
       // return previous vnode to prevent render error causing blank component
       vnode = vm._vnode
     }

Diff for: src/core/observer/watcher.js

@@ -1,14 +1,15 @@
 /* @flow */
 
-import config from '../config'
-import Dep, { pushTarget, popTarget } from './dep'
 import { queueWatcher } from './scheduler'
+import Dep, { pushTarget, popTarget } from './dep'
+
 import {
   warn,
   remove,
   isObject,
   parsePath,
-  _Set as Set
+  _Set as Set,
+  handleError
 } from '../util/index'
 
 let uid = 0
@@ -89,7 +90,17 @@ export default class Watcher {
    */
   get () {
     pushTarget(this)
-    const value = this.getter.call(this.vm, this.vm)
+    let value
+    const vm = this.vm
+    if (this.user) {
+      try {
+        value = this.getter.call(vm, vm)
+      } catch (e) {
+        handleError(e, vm, `getter for watcher "${this.expression}"`)
+      }
+    } else {
+      value = this.getter.call(vm, vm)
+    }
     // "touch" every property so they are all tracked as
     // dependencies for deep watching
     if (this.deep) {
@@ -172,16 +183,7 @@ export default class Watcher {
           try {
             this.cb.call(this.vm, value, oldValue)
           } catch (e) {
-            /* istanbul ignore else */
-            if (config.errorHandler) {
-              config.errorHandler.call(null, e, this.vm)
-            } else {
-              process.env.NODE_ENV !== 'production' && warn(
-                `Error in watcher "${this.expression}"`,
-                this.vm
-              )
-              throw e
-            }
+            handleError(e, this.vm, `callback for watcher "${this.expression}"`)
           }
         } else {
           this.cb.call(this.vm, value, oldValue)

Diff for: src/core/util/error.js

@@ -0,0 +1,15 @@
+import config from '../config'
+import { warn } from './debug'
+
+export function handleError (err, vm, type) {
+  if (config.errorHandler) {
+    config.errorHandler.call(null, err, vm, type)
+  } else {
+    if (process.env.NODE_ENV !== 'production') {
+      warn(`Error in ${type}:`, vm)
+    }
+    if (typeof console !== 'undefined') {
+      console.error(err)
+    }
+  }
+}

Diff for: src/core/util/index.js

@@ -4,4 +4,5 @@ export * from './env'
 export * from './options'
 export * from './debug'
 export * from './props'
+export * from './error'
 export { defineReactive } from '../observer/index'

Diff for: test/helpers/to-have-been-warned.js

@@ -22,8 +22,7 @@ function hasWarned (msg) {
   }
 
   function containsMsg (arg) {
-    if (arg instanceof Error) throw arg
-    return typeof arg === 'string' && arg.indexOf(msg) > -1
+    return arg.toString().indexOf(msg) > -1
   }
 }
 
@@ -52,7 +51,7 @@ beforeEach(() => {
 })
 
 afterEach(done => {
-  const warned = msg => asserted.some(assertedMsg => msg.indexOf(assertedMsg) > -1)
+  const warned = msg => asserted.some(assertedMsg => msg.toString().indexOf(assertedMsg) > -1)
   let count = console.error.calls.count()
   let args
   while (count--) {

Diff for: test/helpers/wait-for-update.js

@@ -13,6 +13,7 @@ import Vue from 'vue'
 // })
 // .then(done)
 window.waitForUpdate = initialCb => {
+  let end
   const queue = initialCb ? [initialCb] : []
 
   function shift () {
@@ -33,13 +34,13 @@ window.waitForUpdate = initialCb => {
           Vue.nextTick(shift)
         }
       }
-    } else if (job && job.fail) {
+    } else if (job && (job.fail || job === end)) {
       job() // done
     }
   }
 
   Vue.nextTick(() => {
-    if (!queue.length || !queue[queue.length - 1].fail) {
+    if (!queue.length || (!end && !queue[queue.length - 1].fail)) {
       throw new Error('waitForUpdate chain is missing .then(done)')
     }
     shift()
@@ -57,6 +58,10 @@ window.waitForUpdate = initialCb => {
       wait.wait = true
       queue.push(wait)
       return chainer
+    },
+    end: endFn => {
+      queue.push(endFn)
+      end = endFn
     }
   }
 

Diff for: test/unit/features/error-handling.spec.js

@@ -0,0 +1,204 @@
+import Vue from 'vue'
+
+const components = createErrorTestComponents()
+
+describe('Error handling', () => {
+  // hooks that prevents the component from rendering, but should not
+  // break parent component
+  ;[
+    ['render', 'render function'],
+    ['beforeCreate', 'beforeCreate hook'],
+    ['created', 'created hook'],
+    ['beforeMount', 'beforeMount hook']
+  ].forEach(([type, description]) => {
+    it(`should recover from errors in ${type}`, done => {
+      const vm = createTestInstance(components[type])
+      expect(`Error in ${description}`).toHaveBeenWarned()
+      expect(`Error: ${type}`).toHaveBeenWarned()
+      assertRootInstanceActive(vm).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)
+    expect(`Error in mounted hook`).toHaveBeenWarned()
+    expect(`Error: mounted`).toHaveBeenWarned()
+    assertBothInstancesActive(vm).then(done)
+  })
+
+  // error in beforeUpdate/updated should affect neither child nor parent
+  ;[
+    ['beforeUpdate', 'beforeUpdate hook'],
+    ['updated', 'updated hook']
+  ].forEach(([type, description]) => {
+    it(`should recover from errors in ${type} hook`, done => {
+      const vm = createTestInstance(components[type])
+      assertBothInstancesActive(vm).then(() => {
+        expect(`Error in ${description}`).toHaveBeenWarned()
+        expect(`Error: ${type}`).toHaveBeenWarned()
+      }).then(done)
+    })
+  })
+
+  ;[
+    ['beforeDestroy', 'beforeDestroy hook'],
+    ['destroyed', 'destroyed hook']
+  ].forEach(([type, description]) => {
+    it(`should recover from errors in ${type} hook`, done => {
+      const vm = createTestInstance(components[type])
+      vm.ok = false
+      waitForUpdate(() => {
+        expect(`Error in ${description}`).toHaveBeenWarned()
+        expect(`Error: ${type}`).toHaveBeenWarned()
+      }).thenWaitFor(next => {
+        assertRootInstanceActive(vm).end(next)
+      }).then(done)
+    })
+  })
+
+  it('should recover from errors in user watcher getter', done => {
+    const vm = createTestInstance(components.userWatcherGetter)
+    vm.n++
+    waitForUpdate(() => {
+      expect(`Error in getter for watcher`).toHaveBeenWarned()
+      function getErrorMsg () {
+        try {
+          this.a.b.c
+        } catch (e) {
+          return e.toString()
+        }
+      }
+      const msg = getErrorMsg.call(vm)
+      expect(msg).toHaveBeenWarned()
+    }).thenWaitFor(next => {
+      assertBothInstancesActive(vm).end(next)
+    }).then(done)
+  })
+
+  it('should recover from errors in user watcher callback', done => {
+    const vm = createTestInstance(components.userWatcherCallback)
+    vm.n++
+    waitForUpdate(() => {
+      expect(`Error in callback for watcher "n"`).toHaveBeenWarned()
+      expect(`Error: userWatcherCallback`).toHaveBeenWarned()
+    }).thenWaitFor(next => {
+      assertBothInstancesActive(vm).end(next)
+    }).then(done)
+  })
+
+  it('config.errorHandler should capture errors', done => {
+    const spy = Vue.config.errorHandler = jasmine.createSpy('errorHandler')
+    const vm = createTestInstance(components.render)
+
+    const args = spy.calls.argsFor(0)
+    expect(args[0].toString()).toContain('Error: render') // error
+    expect(args[1]).toBe(vm.$refs.child) // vm
+    expect(args[2]).toContain('render function') // description
+
+    assertRootInstanceActive(vm).then(done)
+  })
+})
+
+function createErrorTestComponents () {
+  const components = {}
+
+  // render error
+  components.render = {
+    render (h) {
+      throw new Error('render')
+    }
+  }
+
+  // lifecycle errors
+  ;['create', 'mount', 'update', 'destroy'].forEach(hook => {
+    // before
+    const before = 'before' + hook.charAt(0).toUpperCase() + hook.slice(1)
+    const beforeComp = components[before] = {
+      props: ['n'],
+      render (h) {
+        return h('div', this.n)
+      }
+    }
+    beforeComp[before] = function () {
+      throw new Error(before)
+    }
+
+    // after
+    const after = hook.replace(/e?$/, 'ed')
+    const afterComp = components[after] = {
+      props: ['n'],
+      render (h) {
+        return h('div', this.n)
+      }
+    }
+    afterComp[after] = function () {
+      throw new Error(after)
+    }
+  })
+
+  // user watcher
+  components.userWatcherGetter = {
+    props: ['n'],
+    created () {
+      this.$watch(function () {
+        return this.n + this.a.b.c
+      }, val => {
+        console.log('user watcher fired: ' + val)
+      })
+    },
+    render (h) {
+      return h('div', this.n)
+    }
+  }
+
+  components.userWatcherCallback = {
+    props: ['n'],
+    watch: {
+      n () {
+        throw new Error('userWatcherCallback error')
+      }
+    },
+    render (h) {
+      return h('div', this.n)
+    }
+  }
+
+  return components
+}
+
+function createTestInstance (Comp) {
+  return new Vue({
+    data: {
+      n: 0,
+      ok: true
+    },
+    render (h) {
+      return h('div', [
+        'n:' + this.n + '\n',
+        this.ok
+          ? h(Comp, { ref: 'child', props: { n: this.n }})
+          : null
+      ])
+    }
+  }).$mount()
+}
+
+function assertRootInstanceActive (vm, chain) {
+  expect(vm.$el.innerHTML).toContain('n:0\n')
+  vm.n++
+  return waitForUpdate(() => {
+    expect(vm.$el.innerHTML).toContain('n:1\n')
+  })
+}
+
+function assertBothInstancesActive (vm) {
+  vm.n = 0
+  return waitForUpdate(() => {
+    expect(vm.$refs.child.$el.innerHTML).toContain('0')
+  }).thenWaitFor(next => {
+    assertRootInstanceActive(vm).then(() => {
+      expect(vm.$refs.child.$el.innerHTML).toContain('1')
+    }).end(next)
+  })
+}