improve error handling

Author: yyx990803

src/router/index.js

@@ -14,6 +14,7 @@ var historyBackends = {
  *                 - {Boolean} abstract (default: false)
  *                 - {Boolean} saveScrollPosition (default: false)
  *                 - {Boolean} transitionOnLoad (default: false)
+ *                 - {Boolean} suppressTransitionError (default: false)
  *                 - {String} root (default: null)
  *                 - {String} linkActiveClass (default: 'v-link-active')
  */
@@ -60,6 +61,7 @@ function Router (options) {
   // other options
   this._saveScrollPosition = !!options.saveScrollPosition
   this._linkActiveClass = options.linkActiveClass || 'v-link-active'
+  this._suppress = !!options.suppressTransitionError
 
   // create history object
   this.mode = this._abstract

src/transition.js

@@ -184,6 +184,7 @@ p.runQueue = function (queue, fn, cb) {
 p.callHook = function (hook, context, cb, expectBoolean, cleanup) {
   var transition = this
   var nextCalled = false
+
   var next = function (data) {
     if (nextCalled) {
       util.warn('transition.next() should be called only once.')
@@ -195,10 +196,23 @@ p.callHook = function (hook, context, cb, expectBoolean, cleanup) {
     }
     cb(data)
   }
+
   var abort = function () {
     cleanup && cleanup()
     transition.abort()
   }
+
+  var onError = function (err) {
+    // cleanup indicates an after-activation hook,
+    // so instead of aborting we just let the transition
+    // finish.
+    cleanup ? next() : abort()
+    if (err && !transition.router._suppress) {
+      util.warn('Uncaught error during transition: ')
+      throw err instanceof Error ? err : new Error(err)
+    }
+  }
+
   // the copied transition object passed to the user.
   var exposed = {
     to: transition.to,
@@ -209,18 +223,23 @@ p.callHook = function (hook, context, cb, expectBoolean, cleanup) {
       transition.redirect.apply(transition, arguments)
     }
   }
-  var res = hook.call(context, exposed)
+  var res
+  try {
+    res = hook.call(context, exposed)
+  } catch (err) {
+    return onError(err)
+  }
   var promise = util.isPromise(res)
   if (expectBoolean) {
     if (typeof res === 'boolean') {
       res ? next() : abort()
     } else if (promise) {
       res.then(function (ok) {
         ok ? next() : abort()
-      }, abort)
+      }, onError)
     }
   } else if (promise) {
-    res.then(next, abort)
+    res.then(next, onError)
   }
 }
 

test/unit/specs/pipeline/activate.js

@@ -1,9 +1,14 @@
 var testUtils = require('../util')
 var test = testUtils.test
 var assertCalls = testUtils.assertCalls
+var routerUtil = require('../../../../src/util')
 
 describe('activate', function () {
 
+  beforeEach(function () {
+    spyOn(routerUtil, 'warn')
+  })
+
   it('sync', function (done) {
     test({
       a: {
@@ -117,11 +122,35 @@ describe('activate', function () {
       // path changes during validation phase
       expect(router.history.currentPath).toBe('/a')
       setTimeout(function () {
-        // but gets reset when validation fails
-        expect(router.app.$el.textContent).toBe('')
-        expect(router.history.currentPath).toBe('/')
+        // activation error should continue transition
+        expect(router.app.$el.textContent).toBe('A ')
+        expect(router.history.currentPath).toBe('/a')
         done()
       }, wait * 2)
     })
   })
+
+  it('error', function (done) {
+    test({
+      a: {
+        activate: function (transition) {
+          throw new Error('oh no')
+        }
+      }
+    }, function (router, calls, emitter) {
+      var errorThrown = jasmine.createSpy()
+      try {
+        router.go('/a')
+      } catch (e) {
+        errorThrown()
+      }
+      expect(routerUtil.warn).toHaveBeenCalled()
+      expect(errorThrown).toHaveBeenCalled()
+      // should complete the transition despite error
+      assertCalls(calls, ['a.activate'])
+      expect(router.app.$el.textContent).toBe('A ')
+      expect(router.history.currentPath).toBe('/a')
+      done()
+    })
+  })
 })

test/unit/specs/pipeline/data.js

@@ -1,9 +1,14 @@
 var testUtils = require('../util')
 var test = testUtils.test
 var assertCalls = testUtils.assertCalls
+var routerUtil = require('../../../../src/util')
 
 describe('data', function () {
 
+  beforeEach(function () {
+    spyOn(routerUtil, 'warn')
+  })
+
   it('initial load', function (done) {
     test({
       data: {
@@ -87,4 +92,26 @@ describe('data', function () {
     })
   })
 
+  it('promise error', function (done) {
+    test({
+      data: {
+        data: function () {
+          return new Promise(function (resolve, reject) {
+            setTimeout(function () {
+              reject()
+            }, wait)
+          })
+        }
+      }
+    }, function (router, calls, emitter) {
+      router.go('/data/hello')
+      assertCalls(calls, ['data.data'])
+      expect(router.app.$el.textContent).toBe('loading...')
+      setTimeout(function () {
+        // should complete the transition despite error
+        expect(router.app.$el.textContent).toBe('')
+        done()
+      }, wait * 2)
+    })
+  })
 })

test/unit/specs/util.js

@@ -25,11 +25,11 @@ exports.test = function (configs, cb) {
     Object.keys(config).forEach(function (hook) {
       var fn = config[hook]
       config[hook] = function (transition) {
+        var event = route + '.' + hook
+        calls.push(event)
         var res = typeof fn === 'function'
           ? fn(transition)
           : fn
-        var event = route + '.' + hook
-        calls.push(event)
         emitter.emit(event)
         return res
       }