Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle Promise rejection in guards (fix #2833) #2838

Closed
wants to merge 12 commits into from
Closed
137 changes: 137 additions & 0 deletions examples/error-handling/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import Vue from 'vue'
import VueRouter from 'vue-router'

Vue.use(VueRouter)

// eslint-disable-next-line no-unused-vars
let asyncMode = true

const logEl = document.querySelector('.log code')

const makeError = (msg) => {
if (asyncMode) {
return new Promise((resolve, reject) => {
reject(new Error(msg))
})
}

throw new Error(msg)
}

// Components
const Home = { template: '<div>Home</div>' }
const BeforeEach = { template: '<div>BeforeEach</div>' }
const AfterEach = { template: '<div>AfterEach</div>' }
const BeforeEnter = {
template: '<div>BeforeRouteEnter</div>',
beforeRouteEnter () {
return makeError('component.BeforeRouteEnter')
}
}
const BeforeUpdate = {
template: '<div>BeforeRouteUpdate</div>',
beforeRouteUpdate () {
return makeError('component.BeforeRouteUpdate')
},
mounted () {
const currentId = +this.$route.params.id || 1
console.log(`change params.id from ${currentId} to ${currentId + 1}`)
this.$router.push({
name: 'component.beforeRouteUpdate',
params: { id: currentId + 1 }
})
}
}
const BeforeLeave = {
template: '<div>BeforeRouteLeave</div>',
data () {
return {
canLeave: false
}
},
beforeRouteLeave (to, from, next) {
if (from.name === 'component.beforeRouteLeave' && !this.canLeave) {
this.canLeave = true
console.log('click twice to leave route')
return makeError('component.beforeRouteLeave')
}

next()
}
}

const router = new VueRouter({
mode: 'history',
base: __dirname,
routes: [
{ path: '/', component: Home, name: 'home' },
{ path: '/before-enter', component: BeforeEnter, name: 'component.beforeRouteEnter' },
{ path: '/before-update/:id', component: BeforeUpdate, name: 'component.beforeRouteUpdate' },
{ path: '/before-leave', component: BeforeLeave, name: 'component.beforeRouteLeave' },
{ path: '/before-each', component: BeforeEach, name: 'router.beforeEach' },
{ path: '/after-each', component: AfterEach, name: 'router.afterEach' }
]
})

router.onError((err) => {
const modeName = asyncMode ? 'async' : 'sync'
logEl.innerText = `${modeName}: ${err.message}`

console.log(
'%c Router.onError - ' + modeName,
'background: #fff; color: #000',
err.message
)
})

router.afterEach(() => {
return makeError('router.afterEach')
})

// Promise same as:
// router.beforeEach(async (to, from, next) => { throw new Error('Async error') })
router.beforeEach((to, from, next) => {
if (to.name === 'router.beforeEach') {
return makeError('router.beforeEach')
}

next()
})

new Vue({
router,
data () {
return {
asyncMode
}
},
computed: {
nameMode () {
return this.asyncMode ? 'async' : 'sync'
}
},
watch: {
asyncMode (val) {
asyncMode = val
}
},
template: `
<div id="app">
<h1>Error Handling</h1>
<strong @click="asyncMode = !asyncMode" :mode="nameMode" style="cursor: pointer">
Open console - <code>{{ nameMode }} (click)</code>
</strong>
<br>
<ul>
<li><router-link to="/">/home</router-link></li>
<li><router-link to="/before-each">/beforeEach</router-link></li>
<li><router-link to="/after-each">/afterEach</router-link></li>
<li><router-link to="/before-enter">/beforeRouteEnter</router-link></li>
<li><router-link to="/before-update/1">/beforeRouteUpdate</router-link></li>
<li><router-link to="/before-leave">/beforeRouteLeave</router-link></li>
</ul>
<br>
<router-view class="view"></router-view>
</div>
`
}).$mount('#app')
7 changes: 7 additions & 0 deletions examples/error-handling/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<!DOCTYPE html>
<link rel="stylesheet" href="/global.css">
<a href="/">&larr; Examples index</a>
<div id="app"></div><br>
<p class="log">router.onError(<code></code>)</p>
<script src="/__build__/shared.chunk.js"></script>
<script src="/__build__/error-handling.js"></script>
1 change: 1 addition & 0 deletions examples/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ <h1>Vue Router Examples</h1>
<li><a href="discrete-components">Discrete Components</a></li>
<li><a href="nested-router">Nested Routers</a></li>
<li><a href="keepalive-view">Keepalive View</a></li>
<li><a href="error-handling">Error handling</a></li>
</ul>
</body>
</html>
51 changes: 32 additions & 19 deletions src/history/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,24 +86,13 @@ export class History {

confirmTransition (route: Route, onComplete: Function, onAbort?: Function) {
const current = this.current
const abort = err => {
if (isError(err)) {
if (this.errorCbs.length) {
this.errorCbs.forEach(cb => { cb(err) })
} else {
warn(false, 'uncaught error during route navigation:')
console.error(err)
}
}
onAbort && onAbort(err)
}
if (
isSameRoute(route, current) &&
// in the case the route map has been dynamically appended to
route.matched.length === current.matched.length
) {
this.ensureURL()
return abort()
return this._abort(null, onAbort)
}

const {
Expand All @@ -128,14 +117,14 @@ export class History {
this.pending = route
const iterator = (hook: NavigationGuard, next) => {
if (this.pending !== route) {
return abort()
return this._abort(null, onAbort)
}
try {
hook(route, current, (to: any) => {
const hookResponse = hook(route, current, (to: any) => {
if (to === false || isError(to)) {
// next(false) -> abort navigation, ensure current URL
this.ensureURL(true)
abort(to)
this._abort(to, onAbort)
} else if (
typeof to === 'string' ||
(typeof to === 'object' && (
Expand All @@ -144,7 +133,7 @@ export class History {
))
) {
// next('/') or next({ path: '/' }) -> redirect
abort()
this._abort(null, onAbort)
if (typeof to === 'object' && to.replace) {
this.replace(to)
} else {
Expand All @@ -155,8 +144,13 @@ export class History {
next(to)
}
})

// Support async/await in guard (#2833)
if (hookResponse instanceof Promise) {
hookResponse.catch(e => this._abort(e, onAbort))
}
} catch (e) {
abort(e)
this._abort(e, onAbort)
}
}

Expand All @@ -169,7 +163,7 @@ export class History {
const queue = enterGuards.concat(this.router.resolveHooks)
runQueue(queue, iterator, () => {
if (this.pending !== route) {
return abort()
return this._abort(null, onAbort)
}
this.pending = null
onComplete(route)
Expand All @@ -187,9 +181,28 @@ export class History {
this.current = route
this.cb && this.cb(route)
this.router.afterHooks.forEach(hook => {
hook && hook(route, prev)
if (hook) {
const hookResponse = hook(route, prev)

// Support async/await in guard (#2833)
if (hookResponse instanceof Promise) {
hookResponse.catch(err => this._abort(err))
}
}
})
}

_abort (err: any, cb: ?Function) {
if (isError(err)) {
if (this.errorCbs.length) {
this.errorCbs.forEach(cb => { cb(err) })
} else {
warn(false, 'uncaught error during route navigation:')
console.error(err)
}
}
cb && cb(err)
}
}

function normalizeBase (base: ?string): string {
Expand Down
71 changes: 71 additions & 0 deletions test/e2e/specs/error-handling.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
const bsStatus = require('../browserstack-send-status')

const PAGE_NAME = 'error-handling'
const BASE_PAGE = `http://localhost:8080/${PAGE_NAME}`

module.exports = {
...bsStatus(),

'@tags': ['history'],

[BASE_PAGE]: function (browser) {
browser
.url(BASE_PAGE + '/')
.waitForElementVisible('#app', 1000)
.assert.count('li a', 6)
// assert correct href with base
.assert.attributeContains('li:nth-child(1) a', 'href', `/${PAGE_NAME}/`)
.assert.attributeContains('li:nth-child(2) a', 'href', `/${PAGE_NAME}/before-each`)
.assert.attributeContains('li:nth-child(3) a', 'href', `/${PAGE_NAME}/after-each`)
.assert.attributeContains('li:nth-child(4) a', 'href', `/${PAGE_NAME}/before-enter`)
.assert.attributeContains('li:nth-child(5) a', 'href', `/${PAGE_NAME}/before-update/1`)
.assert.attributeContains('li:nth-child(6) a', 'href', `/${PAGE_NAME}/before-leave`)
.assert.attributeContains('strong', 'mode', 'async')
.assert.containsText('.log code', 'async: router.afterEach')
.assert.containsText('.view', 'Home')

// Error on enter (global)
.click('li:nth-child(2) a')
.assert.urlEquals(`${BASE_PAGE}/`)
.assert.containsText('.log code', 'async: router.beforeEach')
.assert.containsText('.view', 'Home')

// Error on leave (global)
.click('li:nth-child(3) a')
.assert.urlEquals(`${BASE_PAGE}/after-each`)
.assert.containsText('.log code', 'async: router.afterEach')
.assert.containsText('.view', 'AfterEach')

// Error on enter (component)
.click('li:nth-child(4) a')
.assert.urlEquals(`${BASE_PAGE}/after-each`)
.assert.containsText('.log code', 'async: component.BeforeRouteEnter')
.assert.containsText('.view', 'AfterEach')

// Error on change route (component)
// mounted change route.id to 2
.click('li:nth-child(5) a')
.assert.urlEquals(`${BASE_PAGE}/before-update/1`)
.assert.containsText('.log code', 'async: component.BeforeRouteUpdate')
.assert.containsText('.view', 'BeforeRouteUpdate')

// Error on leave route (component)
.click('li:nth-child(6) a')
.assert.urlEquals(`${BASE_PAGE}/before-leave`)
.assert.containsText('.log code', 'async: router.afterEach')
.assert.containsText('.view', 'BeforeRouteLeave')

// Click twice to leave the route
.click('li:nth-child(1) a')
.assert.urlEquals(`${BASE_PAGE}/before-leave`)
.assert.containsText('.log code', 'async: component.beforeRouteLeave')
.assert.containsText('.view', 'BeforeRouteLeave')

.click('li:nth-child(1) a')
.assert.urlEquals(`${BASE_PAGE}/`)
.assert.containsText('.log code', 'async: router.afterEach')
.assert.containsText('.view', 'Home')

.end()
}
}
50 changes: 50 additions & 0 deletions test/unit/specs/error-handling.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,54 @@ describe('error handling', () => {
expect(spy1).toHaveBeenCalledWith(err)
expect(spy2).toHaveBeenCalledWith(err)
})

// #2833
// async/await => router.beforeEach(async () => { throw err })
// Promise => router.beforeEach(() => new Promise((resolve, reject) => reject(err)))
describe('async/await, handle onError', () => {
describe('Global', () => {
let router, err, spy

beforeEach(() => {
router = new VueRouter()
err = new Error('foo')
spy = jasmine.createSpy('error')
router.onError(spy)
})

const promiseError = () => new Promise((resolve, reject) => {
reject(err)
})

it('beforeEach', () => {
router.beforeEach(() => promiseError())

router.push('/foo', () => {
fail('onError function did not receive an error')
}, () => {
expect(spy).toHaveBeenCalledWith(err)
})
})

it('afterEach', () => {
router.afterEach(() => promiseError())

router.push('/foo', () => {
Vue.nextTick(() => expect(spy).toHaveBeenCalledWith(err))
}, () => {
fail('onError function did not receive an error')
})
})

it('beforeResolve', () => {
router.beforeResolve(() => promiseError())

router.push('/foo', () => {
fail('onError function did not receive an error')
}, () => {
expect(spy).toHaveBeenCalledWith(err)
})
})
})
})
})