From 04f7a95a5a9239971239925d73b59433af7109c5 Mon Sep 17 00:00:00 2001 From: Louis-Marie Michelin Date: Sun, 8 Dec 2019 17:01:41 +0100 Subject: [PATCH 1/3] test: add example for navigation errors --- examples/index.html | 1 + examples/router-errors/app.js | 53 +++++++++++++++++++++++++++++++ examples/router-errors/index.html | 8 +++++ 3 files changed, 62 insertions(+) create mode 100644 examples/router-errors/app.js create mode 100644 examples/router-errors/index.html diff --git a/examples/index.html b/examples/index.html index c0ee5bc16..f98b9e7d1 100644 --- a/examples/index.html +++ b/examples/index.html @@ -18,6 +18,7 @@

Vue Router Examples

  • Route Props
  • Route Alias
  • Route Params
    • +
  • Router errors
  • Transitions
  • Data Fetching
  • Navigation Guards
    • diff --git a/examples/router-errors/app.js b/examples/router-errors/app.js new file mode 100644 index 000000000..749d36463 --- /dev/null +++ b/examples/router-errors/app.js @@ -0,0 +1,53 @@ +import Vue from 'vue' +import VueRouter from 'vue-router' + +const component = { + template: ` +
    + {{ $route.fullPath }} +
    + ` +} + +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') diff --git a/examples/router-errors/index.html b/examples/router-errors/index.html new file mode 100644 index 000000000..12011560b --- /dev/null +++ b/examples/router-errors/index.html @@ -0,0 +1,8 @@ + +
    + / + /foo + +
    + + From 449f783ba1c8869ce4d85897e78e8d246ebcdd7c Mon Sep 17 00:00:00 2001 From: Louis-Marie Michelin Date: Sun, 8 Dec 2019 17:01:49 +0100 Subject: [PATCH 2/3] test: add unit tests for navigation errors --- test/unit/specs/error-handling.spec.js | 50 ++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/unit/specs/error-handling.spec.js b/test/unit/specs/error-handling.spec.js index 9daf5b172..01a0e8989 100644 --- a/test/unit/specs/error-handling.spec.js +++ b/test/unit/specs/error-handling.spec.js @@ -40,6 +40,56 @@ describe('error handling', () => { }) }) + it('NavigationDuplicated error', done => { + const router = new VueRouter() + + router.push('/foo') + router.push('/foo').catch(err => { + expect(err._type).toBe('NavigationDuplicated') + 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('NavigationCancelled') + 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('NavigationRedirected') + 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('NavigationAborted') + done() + }) + }) + it('async component errors', done => { spyOn(console, 'warn') const err = new Error('foo') From 179dee4f912b509747aa1945f263fbae8918d5ba Mon Sep 17 00:00:00 2001 From: Louis-Marie Michelin Date: Mon, 18 May 2020 21:57:08 +0200 Subject: [PATCH 3/3] feat(errors): create router errors --- src/history/abstract.js | 6 +- src/history/base.js | 23 ++++--- src/history/errors.js | 85 ++++++++++++++++++++------ src/util/warn.js | 8 +-- test/unit/specs/error-handling.spec.js | 9 +-- 5 files changed, 91 insertions(+), 40 deletions(-) diff --git a/src/history/abstract.js b/src/history/abstract.js index 7b6bdf2a8..9437182e7 100644 --- a/src/history/abstract.js +++ b/src/history/abstract.js @@ -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 @@ -51,7 +51,7 @@ export class AbstractHistory extends History { this.updateRoute(route) }, err => { - if (isExtendedError(NavigationDuplicated, err)) { + if (isRouterError(err, NavigationFailureType.NAVIGATION_DUPLICATED)) { this.index = targetIndex } } diff --git a/src/history/base.js b/src/history/base.js index 2b12e1967..cf79e2846 100644 --- a/src/history/base.js +++ b/src/history/base.js @@ -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 @@ -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)) { if (this.errorCbs.length) { this.errorCbs.forEach(cb => { cb(err) @@ -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( @@ -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 ( @@ -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 { diff --git a/src/history/errors.js b/src/history/errors.js index 1d2118c57..4fac5725a 100644 --- a/src/history/errors.js +++ b/src/history/errors.js @@ -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) +} diff --git a/src/util/warn.js b/src/util/warn.js index 9e8982fb6..73e70caf8 100644 --- a/src/util/warn.js +++ b/src/util/warn.js @@ -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) } diff --git a/test/unit/specs/error-handling.spec.js b/test/unit/specs/error-handling.spec.js index 01a0e8989..8d41bca18 100644 --- a/test/unit/specs/error-handling.spec.js +++ b/test/unit/specs/error-handling.spec.js @@ -1,5 +1,6 @@ import Vue from 'vue' import VueRouter from '../../../src/index' +import { NavigationFailureType } from '../../../src/history/errors' Vue.use(VueRouter) @@ -45,7 +46,7 @@ describe('error handling', () => { router.push('/foo') router.push('/foo').catch(err => { - expect(err._type).toBe('NavigationDuplicated') + expect(err.type).toBe(NavigationFailureType.duplicated) done() }) }) @@ -58,7 +59,7 @@ describe('error handling', () => { }) router.push('/foo').catch(err => { - expect(err._type).toBe('NavigationCancelled') + expect(err.type).toBe(NavigationFailureType.cancelled) done() }) router.push('/') @@ -74,7 +75,7 @@ describe('error handling', () => { }) router.push('/foo?redirect=/').catch(err => { - expect(err._type).toBe('NavigationRedirected') + expect(err.type).toBe(NavigationFailureType.redirected) done() }) }) @@ -85,7 +86,7 @@ describe('error handling', () => { router.beforeEach((to, from, next) => { next(false) }) router.push('/foo').catch(err => { - expect(err._type).toBe('NavigationAborted') + expect(err.type).toBe(NavigationFailureType.aborted) done() }) })