Create router errors #3047

lmichelin · 2019-11-30T16:02:24Z

This PR improves the way router errors are thrown.

4 router errors types have been created:

NavigationDuplicated
NavigationCancelled
NavigationRedirected
NavigationAborted

Before:

Uncaught (in promise) undefined

After:

Uncaught (in promise) Error: NavigationAborted: Navigating to location ("/foo?abort=y") has been aborted
    at eval (base.js:157)
    at eval (app.js:33)
    at iterator (base.js:153)
    at step (async.js:13)
    at step (async.js:17)
    at runQueue (async.js:21)
    at HashHistory.confirmTransition (base.js:179)
    at HashHistory.transitionTo (base.js:74)

lmichelin · 2019-12-08T16:18:21Z

@posva I wrote some unit tests, the PR is ready to be reviewed 😉

ariabuckles · 2020-03-05T23:42:03Z

Bump on this? This would be really helpful for tracking down where promise rejections are coming from <3

bponomarenko · 2020-03-11T13:47:15Z

It seems this PR can also solve #3027. +1 for it to be merged and released.

quentinus95 · 2020-03-17T08:16:17Z

@lmichelin could it be possible to export the exceptions so they can be imported (e.g. using import { NavigationDuplicated } from "vue-router")? It would allow catching the exceptions and use instanceof (especially when coding with TypeScript) to take decisions based on the thrown exceptions.

posva · 2020-03-17T08:33:01Z

@quentinus95 instanceof with extended Errors doesn't work on some browsers like IE.
This PR is on hold until vuejs/router#123 is merged to make sure exported errors share the same naming and reduce migration cost

posva

Hi @lmichelin

Now that errors are stable on vue-router-next, I think we could try to align the user API part to make migration easier.
Namely, we need:

a type, from property
an object (to simulate the enumeration) with the same properties as NavigationFailureType
an exported TS interface so users can type errors

We can also remove:

the name and _name properties
all the code to hack around Error extension and use plain errors (https://github.com/vuejs/vue-router-next/blob/master/src/errors.ts#L77-L90)

It's okay to have a few differences like the lack of from and to properties. Also, let me know if you don't have the time so I can pick it up. If you only manage to add a few things but not all, that's, of course, fine as well!

Link to RFC: vuejs/rfcs#150
Link to vue-router-next errors file: https://github.com/vuejs/vue-router-next/blob/master/src/errors.ts

package.json

src/history/abstract.js

src/history/base.js

src/history/errors.js

posva

I realised you have a sample for an e2e test but it doesn't seem like you need it since you added a unit test already

src/util/warn.js

src/history/base.js

src/history/errors.js

lmichelin · 2020-05-18T19:44:18Z

I realised you have a sample for an e2e test but it doesn't seem like you need it since you added a unit test already

examples/router-errors/app.js is an example to show how to throw the errors, it's not made for e2e purposes, but just to show simple examples for these errors.

posva

Thanks a lot for your help with this @lmichelin and sorry for the delay!

schmidtk

Good morning @lmichelin and @posva. I discovered this PR while handling navigation duplicated errors in my application and have been tracking the changes in case it requires migration when we update vue-router in the future.

I noted two places in the changes where NavigationFailureType.NAVIGATION_DUPLICATED references were not updated after the keys were changed in NavigationFailureType. I'm admittedly new to vue-router internals, so apologies if I misunderstood something here. Thanks for your time.

schmidtk · 2020-05-19T13:00:54Z

src/history/base.js

@@ -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)) {


This should be renamed to NavigationFailureType.duplicated.

schmidtk · 2020-05-19T13:01:17Z

src/history/abstract.js

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


Here as well.

lmichelin · 2020-05-19T13:10:56Z

You are right, I completely forgot these references 🙈

lmichelin · 2020-05-19T13:39:42Z

I just opened a new PR to fix this: #3204

alecgibson · 2020-05-29T10:19:20Z

I think this was a breaking change? We don't get the name property on the Error any more, which is useful when trying to handle particular errors (eg ignoring NavigationDuplicated). At the very least it may be nice to expose isRouterError as a utility method? And also to expose the failure types, especially because it's an opaque enum.

posva · 2020-05-29T10:34:25Z

The errors were not documented and considered private. Checking navigation failures docs are on the way!

alecgibson · 2020-05-29T10:37:16Z

I'd say that checking an Error.name field is pretty standard practice, especially if it's not been prefixed with an underscore. I don't mind a change, but I'm just saying this could be considered breaking.

snoozbuster · 2020-06-10T02:33:26Z

+1 for this being a breaking change - once again I am back here to figure out why suddenly there are errors in my app where there weren't errors before (and the fact that it was merged and published without docs on how to do proper error checking also seems like a mistake)

snoozbuster · 2020-06-10T03:15:02Z

@posva - are these type constants exported from the package, so that I can inspect navigation errors in my application and handle them appropriately? I don't see anything which adds that, is it in another PR?

afv · 2020-06-16T16:45:03Z

I was checking err.name === 'NavigationDuplicated' before but after these changes the name is just 'Error'.

As a workaround I'm now importing NavigationFailureType at main.js and adding it to window:

import { NavigationFailureType } from 'vue-router/src/history/errors';

window.NavigationFailureType = NavigationFailureType;

On components I can now check err.type === window.NavigationFailureType.duplicated.

posva · 2020-06-16T16:54:27Z

Please check #3220 for the proposed documentation for errors. Your feedback could be valuable there

posva mentioned this pull request Jan 5, 2020

beforeEnter (node:15088) UnhandledPromiseRejectionWarning: undefined #3089

Closed

jonatahr mentioned this pull request Jan 8, 2020

Export NavigationDuplicated class #3039

Closed

posva added this to Long term road (high prio, low complex) in Longterm Jan 14, 2020

posva mentioned this pull request Jan 31, 2020

Throw proper error when aborting navigation #3113

Closed

posva mentioned this pull request Feb 13, 2020

Aborting navigation throws undefined #3122

Closed

bponomarenko mentioned this pull request Mar 16, 2020

Add onComplete & onAbort to router-link #3027

Open

bponomarenko mentioned this pull request Mar 27, 2020

Add possibility to handle aborted navigations globally #3157

Closed

posva requested changes May 11, 2020

View reviewed changes

posva requested changes May 14, 2020

View reviewed changes

lmichelin added 2 commits May 16, 2020 11:58

test: add example for navigation errors

04f7a95

test: add unit tests for navigation errors

449f783

posva requested changes May 16, 2020

View reviewed changes

src/util/warn.js Outdated Show resolved Hide resolved

src/history/base.js Show resolved Hide resolved

src/history/errors.js Outdated Show resolved Hide resolved

feat(errors): create router errors

179dee4

posva approved these changes May 19, 2020

View reviewed changes

posva merged commit 4c727f9 into vuejs:dev May 19, 2020

schmidtk reviewed May 19, 2020

View reviewed changes

lmichelin mentioned this pull request May 19, 2020

fix: Missing renaming #3204

Merged

posva moved this from Long term road (high prio, low complex) to Done in Longterm May 19, 2020

lbwa mentioned this pull request May 29, 2020

fix(errors): NavigationCanceled with async components #3211

Merged

dmitry mentioned this pull request Jun 3, 2020

SSR request raises error 500 after upgrade to vue-router 3.3.0 on beforeEach redirection quasarframework/quasar#7152

Closed

posva mentioned this pull request Jun 11, 2020

Navigation guard redirect causes errors when navigating programmatically #3224

Closed

trygveaa mentioned this pull request Jun 17, 2020

docs: add navigation failures #3220

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create router errors #3047

Create router errors #3047

lmichelin commented Nov 30, 2019 •

edited

lmichelin commented Dec 8, 2019

ariabuckles commented Mar 5, 2020

bponomarenko commented Mar 11, 2020

quentinus95 commented Mar 17, 2020 •

edited

posva commented Mar 17, 2020

posva left a comment

posva left a comment

lmichelin commented May 18, 2020

posva left a comment

schmidtk left a comment

schmidtk May 19, 2020

schmidtk May 19, 2020

lmichelin commented May 19, 2020

lmichelin commented May 19, 2020

alecgibson commented May 29, 2020 •

edited

posva commented May 29, 2020 •

edited

alecgibson commented May 29, 2020

snoozbuster commented Jun 10, 2020 •

edited

snoozbuster commented Jun 10, 2020

afv commented Jun 16, 2020

posva commented Jun 16, 2020

Create router errors #3047

Create router errors #3047

Conversation

lmichelin commented Nov 30, 2019 • edited

lmichelin commented Dec 8, 2019

ariabuckles commented Mar 5, 2020

bponomarenko commented Mar 11, 2020

quentinus95 commented Mar 17, 2020 • edited

posva commented Mar 17, 2020

posva left a comment

Choose a reason for hiding this comment

posva left a comment

Choose a reason for hiding this comment

lmichelin commented May 18, 2020

posva left a comment

Choose a reason for hiding this comment

schmidtk left a comment

Choose a reason for hiding this comment

schmidtk May 19, 2020

Choose a reason for hiding this comment

schmidtk May 19, 2020

Choose a reason for hiding this comment

lmichelin commented May 19, 2020

lmichelin commented May 19, 2020

alecgibson commented May 29, 2020 • edited

posva commented May 29, 2020 • edited

alecgibson commented May 29, 2020

snoozbuster commented Jun 10, 2020 • edited

snoozbuster commented Jun 10, 2020

afv commented Jun 16, 2020

posva commented Jun 16, 2020

lmichelin commented Nov 30, 2019 •

edited

quentinus95 commented Mar 17, 2020 •

edited

alecgibson commented May 29, 2020 •

edited

posva commented May 29, 2020 •

edited

snoozbuster commented Jun 10, 2020 •

edited