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

Create router errors #3047

Merged
merged 3 commits into from
May 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ <h1>Vue Router Examples</h1>
<li><a href="route-props">Route Props</a></li>
<li><a href="route-alias">Route Alias</a></li>
<li><a href="route-params">Route Params</a></li>
<li><a href="router-errors">Router errors</a></li>
<li><a href="transitions">Transitions</a></li>
<li><a href="data-fetching">Data Fetching</a></li>
<li><a href="navigation-guards">Navigation Guards</a></li>
Expand Down
53 changes: 53 additions & 0 deletions examples/router-errors/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import Vue from 'vue'
import VueRouter from 'vue-router'

const component = {
template: `
<div>
{{ $route.fullPath }}
</div>
`
}

Vue.use(VueRouter)

const router = new VueRouter({
routes: [
{ path: '/', component }, { path: '/foo', component }
]
})

router.beforeEach((to, from, next) => {
console.log('from', from.fullPath)
console.log('going to', to.fullPath)
if (to.query.wait) {
setTimeout(() => next(), 100)
} else if (to.query.redirect) {
next(to.query.redirect)
} else if (to.query.abort) {
next(false)
} else {
next()
}
})

new Vue({
el: '#app',
router
})

// 4 NAVIGATION ERROR CASES :

// navigation duplicated
// router.push('/foo')
// router.push('/foo')

// navigation cancelled
// router.push('/foo?wait=y')
// router.push('/')

// navigation redirected
// router.push('/foo?redirect=/')

// navigation aborted
// router.push('/foo?abort=y')
8 changes: 8 additions & 0 deletions examples/router-errors/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!DOCTYPE html>
<div id="app">
<router-link to="/">/</router-link>
<router-link to="/foo">/foo</router-link>
<router-view></router-view>
</div>
<script src="/__build__/shared.chunk.js"></script>
<script src="/__build__/router-errors.js"></script>
6 changes: 3 additions & 3 deletions src/history/abstract.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

import type Router from '../index'
import { History } from './base'
import { NavigationDuplicated } from './errors'
import { isExtendedError } from '../util/warn'
import { isRouterError } from '../util/warn'
import { NavigationFailureType } from './errors'

export class AbstractHistory extends History {
index: number
Expand Down Expand Up @@ -51,7 +51,7 @@ export class AbstractHistory extends History {
this.updateRoute(route)
},
err => {
if (isExtendedError(NavigationDuplicated, err)) {
if (isRouterError(err, NavigationFailureType.NAVIGATION_DUPLICATED)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here as well.

this.index = targetIndex
}
}
Expand Down
23 changes: 16 additions & 7 deletions src/history/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,20 @@ import { _Vue } from '../install'
import type Router from '../index'
import { inBrowser } from '../util/dom'
import { runQueue } from '../util/async'
import { warn, isError, isExtendedError } from '../util/warn'
import { warn, isError, isRouterError } from '../util/warn'
import { START, isSameRoute } from '../util/route'
import {
flatten,
flatMapComponents,
resolveAsyncComponents
} from '../util/resolve-components'
import { NavigationDuplicated } from './errors'
import {
createNavigationDuplicatedError,
createNavigationCancelledError,
createNavigationRedirectedError,
createNavigationAbortedError,
NavigationFailureType
} from './errors'

export class History {
router: Router
Expand Down Expand Up @@ -104,7 +110,7 @@ export class History {
// When the user navigates through history through back/forward buttons
// we do not want to throw the error. We only throw it if directly calling
// push/replace. That's why it's not included in isError
if (!isExtendedError(NavigationDuplicated, err) && isError(err)) {
if (!isRouterError(err, NavigationFailureType.NAVIGATION_DUPLICATED) && isError(err)) {
posva marked this conversation as resolved.
Show resolved Hide resolved

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be renamed to NavigationFailureType.duplicated.

if (this.errorCbs.length) {
this.errorCbs.forEach(cb => {
cb(err)
Expand All @@ -122,7 +128,7 @@ export class History {
route.matched.length === current.matched.length
) {
this.ensureURL()
return abort(new NavigationDuplicated(route))
return abort(createNavigationDuplicatedError(current, route))
}

const { updated, deactivated, activated } = resolveQueue(
Expand All @@ -146,12 +152,15 @@ export class History {
this.pending = route
const iterator = (hook: NavigationGuard, next) => {
if (this.pending !== route) {
return abort()
return abort(createNavigationCancelledError(current, route))
}
try {
hook(route, current, (to: any) => {
if (to === false || isError(to)) {
if (to === false) {
// next(false) -> abort navigation, ensure current URL
this.ensureURL(true)
abort(createNavigationAbortedError(current, route))
} else if (isError(to)) {
this.ensureURL(true)
abort(to)
} else if (
Expand All @@ -160,7 +169,7 @@ export class History {
(typeof to.path === 'string' || typeof to.name === 'string'))
) {
// next('/') or next({ path: '/' }) -> redirect
abort()
abort(createNavigationRedirectedError(current, route))
if (typeof to === 'object' && to.replace) {
this.replace(to)
} else {
Expand Down
85 changes: 65 additions & 20 deletions src/history/errors.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,67 @@
export class NavigationDuplicated extends Error {
constructor (normalizedLocation) {
super()
this.name = this._name = 'NavigationDuplicated'
// passing the message to super() doesn't seem to work in the transpiled version
this.message = `Navigating to current location ("${
normalizedLocation.fullPath
}") is not allowed`
// add a stack property so services like Sentry can correctly display it
Object.defineProperty(this, 'stack', {
value: new Error().stack,
writable: true,
configurable: true
})
// we could also have used
// Error.captureStackTrace(this, this.constructor)
// but it only exists on node and chrome
}
export const NavigationFailureType = {
redirected: 1,
aborted: 2,
cancelled: 3,
duplicated: 4
}

export function createNavigationRedirectedError (from, to) {
return createRouterError(
from,
to,
NavigationFailureType.redirected,
`Redirected from "${from.fullPath}" to "${stringifyRoute(to)}" via a navigation guard.`
)
}

export function createNavigationDuplicatedError (from, to) {
return createRouterError(
from,
to,
NavigationFailureType.duplicated,
`Avoided redundant navigation to current location: "${from.fullPath}".`
)
}

export function createNavigationCancelledError (from, to) {
return createRouterError(
from,
to,
NavigationFailureType.cancelled,
`Navigation cancelled from "${from.fullPath}" to "${to.fullPath}" with a new navigation.`
)
}

// support IE9
NavigationDuplicated._name = 'NavigationDuplicated'
export function createNavigationAbortedError (from, to) {
return createRouterError(
from,
to,
NavigationFailureType.aborted,
`Navigation aborted from "${from.fullPath}" to "${to.fullPath}" via a navigation guard.`
)
}

function createRouterError (from, to, type, message) {
const error = new Error(message)
error._isRouter = true
error.from = from
error.to = to
error.type = type

const newStack = error.stack.split('\n')
newStack.splice(1, 2) // remove 2 last useless calls
error.stack = newStack.join('\n')
return error
}

const propertiesToLog = ['params', 'query', 'hash']

function stringifyRoute (to) {
if (typeof to === 'string') return to
if ('path' in to) return to.path
const location = {}
for (const key of propertiesToLog) {
if (key in to) location[key] = to[key]
}
return JSON.stringify(location, null, 2)
}
8 changes: 2 additions & 6 deletions src/util/warn.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ export function isError (err: any): boolean {
return Object.prototype.toString.call(err).indexOf('Error') > -1
}

export function isExtendedError (constructor: Function, err: any): boolean {
return (
err instanceof constructor ||
// _name is to support IE9 too
(err && (err.name === constructor.name || err._name === constructor._name))
)
export function isRouterError (err: any, errorType: ?string): boolean {
return isError(err) && err._isRouter && (errorType == null || err.type === errorType)
}
51 changes: 51 additions & 0 deletions test/unit/specs/error-handling.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Vue from 'vue'
import VueRouter from '../../../src/index'
import { NavigationFailureType } from '../../../src/history/errors'

Vue.use(VueRouter)

Expand Down Expand Up @@ -40,6 +41,56 @@ describe('error handling', () => {
})
})

it('NavigationDuplicated error', done => {
const router = new VueRouter()

router.push('/foo')
router.push('/foo').catch(err => {
expect(err.type).toBe(NavigationFailureType.duplicated)
done()
})
})

it('NavigationCancelled error', done => {
const router = new VueRouter()

router.beforeEach((to, from, next) => {
setTimeout(() => next(), 100)
})

router.push('/foo').catch(err => {
expect(err.type).toBe(NavigationFailureType.cancelled)
done()
})
router.push('/')
})

it('NavigationRedirected error', done => {
const router = new VueRouter()

router.beforeEach((to, from, next) => {
if (to.query.redirect) {
next(to.query.redirect)
}
})

router.push('/foo?redirect=/').catch(err => {
expect(err.type).toBe(NavigationFailureType.redirected)
done()
})
})

it('NavigationAborted error', done => {
const router = new VueRouter()

router.beforeEach((to, from, next) => { next(false) })

router.push('/foo').catch(err => {
expect(err.type).toBe(NavigationFailureType.aborted)
done()
})
})

it('async component errors', done => {
spyOn(console, 'warn')
const err = new Error('foo')
Expand Down