vue-router: $router.push() followed by redirect in a route's beforeEnter hook causes uncaught promise rejection

@posva Thanks for the clarification! However, I would argue that a redirection that cancels a navigation should not be a Promise rejection. Since rejection becomes a throw in the async/await model, it should only occur on actual errors. In my opinion, an expected control flow with a redirection should not have to add error handlers to avoid errors. While reading #2881 and the relevant documentation again, I could not find a description of what $router.push() actually resolves to; only that it should resolve (or reject) when navigation is complete.

The example code you gave in #2881 inspired me to create this global push() override:

const router = new VueRouter({ /* ... */ });

const originalPush = router.push;
router.push = function push(location, onResolve, onReject) {
  if (onResolve || onReject) {
    return originalPush.call(this, location, onResolve, onReject);
  }
  return originalPush.call(this, location).catch((err) => {
    if (err) {
      // If there really is an error, throw it
      // Should probably be more sophisticated based on the type of err
      return Promise.reject(err);
    }
    // Otherwise resolve to false to indicate the original push call didn't go to its original
    // destination.
    // TODO: return the final route instead of false?
    return Promise.resolve(false);
  });
};

Since the precise resolve/reject behavior of push() doesn’t seem set in stone, would it be possible to resolve to the final route (or false if navigation aborted) instead of rejecting when there is no error?

I already answered this at https://github.com/vuejs/vue-router/issues/2833#issuecomment-532158403

@berniegp unfortunately, this feature request will do nothing to help you with that. It’s is related to the new promise api and errors are stil being improved (there were not before). You can find more at #2881 (comment)

Please, check the related issues, the reasoning has been verbosely explained there already

@posva I did read your previous reply and the referenced issue #2881 in its entirety. Here’s why I thought this issue is different in more detail:

1. NavigationDuplicated is mentioned all over the place in #2881 but it is not involved at all in my jsfiddle. For reference, here is the stack trace from my jsfiddle in Chrome:

   Uncaught (in promise) undefined
   (anonymous) @ vue-router.js:2057
   abort @ vue-router.js:2088
   (anonymous) @ vue-router.js:2137
   beforeEnter @ Inline Babel script:25
   iterator @ vue-router.js:2126
   step @ vue-router.js:1852
   step @ vue-router.js:1856
   runQueue @ vue-router.js:1860
   confirmTransition @ vue-router.js:2153
   transitionTo @ vue-router.js:2040
   push @ vue-router.js:2371
   (anonymous) @ vue-router.js:2785
   push @ vue-router.js:2784
   click @ Inline Babel script:10
   

   And the code at abort @ vue-router.js:2088:

   // base.js
   
   var abort = function (err) {
     // after merging https://github.com/vuejs/vue-router/pull/2771 we
     // 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 (this$1.errorCbs.length) {
         this$1.errorCbs.forEach(function (cb) {
           cb(err);
         });
       } else {
         warn(false, 'uncaught error during route navigation:');
         console.error(err);
       }
     }
     onAbort && onAbort(err); // <-- Line 2088, note that err === undefined
                              // in my case so *not* a NavigationDuplicated instance
   };
   

2. My understanding is that the whole discussion in #2881 is centered around error handling. My use-case does not involve any error-generating code path. Here is where the abort() is triggered in confirmTransition():

   // base.js
   
   const iterator = (hook: NavigationGuard, next) => {
     if (this.pending !== route) {
       return abort()
     }
     try {
       hook(route, current, (to: any) => {
         if (to === false || isError(to)) {
           // next(false) -> abort navigation, ensure current URL
           this.ensureURL(true)
           abort(to)
         } else if (
           typeof to === 'string' ||
           (typeof to === 'object' &&
             (typeof to.path === 'string' || typeof to.name === 'string'))
         ) {
           // next('/') or next({ path: '/' }) -> redirect
           abort() // <----------- This causes onAbort to be called which is the Promise's
                   //              reject function passed from VueRouter.prototype.push()
           if (typeof to === 'object' && to.replace) {
             this.replace(to)
           } else {
             this.push(to)
           }
         } else {
           // confirm transition and pass on the value
           next(to)
         }
       })
     } catch (e) {
       abort(e)
     }
   }
   

3. I understood your comment in #2833 (comment) as telling me that my issue was separate from #2833.

I spent quite some time debugging, researching and reproducing this in a minimal test case and I honestly thought this issue was different from the others linked. Note also that, in an attempt to not add noise to the repo, my first step was to find another related issue and not hastily create a new one 😃. Of course all these issues are related to Promise rejection so maybe they will all be fixed the same way.

Why is this issue still closed? The “supposedly” related issues where closed but this is still valid (which show they weren’t the same) The proposed solution of @berniegp works, so please reopen the issue and review the proposal to either allow a PR or reject it with at least some feedback.

This error is really annoying 👎 this is the first time I see something like this, and it ends having all kind of reverse situations if you want to check auth before the call 😦

If got this Error in my console: Uncaught (in promise) TypeError: Cannot read property 'toString' of undefined

After a while of debugging, this is due to the abort call described above with undefined as error.

Maybe a better solution is throwing a real error like NavigationCanceled or something.

Calling router push with empty function fixed it magically: $router.push('/', () => {})

Have the same problem. I have another question I have big project and now I should implement this $router.push(‘/’, () => {}) everywhere why? @berniegp good solution but why it is not in core package?

Any navigation failure is a promise rejection. I updated my comment at https://github.com/vuejs/vue-router/issues/2881#issuecomment-520554378 that explains everything. Let me know if there is anything to add

Some errors haven’t been changed yet and that’s why the uncaught error may be undefined or false in some scenarios