ember.js icon indicating copy to clipboard operation
ember.js copied to clipboard

Router is in invalid state after transitionTo: TypeError: Cannot read property 'name' of undefined

Open st-h opened this issue 5 years ago • 4 comments

Our ember app uses the application routes error hook to redirect to a 404 template:

actions: {
    error: function(error/*, transition*/) {
      const status = error.status;
      if (status == 404) {
        this.intermediateTransitionTo('404');
        return false;
      }
      ...
    }
  }

and (this seems to be rather important) a 404 route, that has a path, which contains a wildcard. So far I was unable to reproduce without a wildcard path.

this.route('404', { path: '/*' });

This causes the ember router to end up in an invalid state:

Uncaught (in promise) TypeError: Cannot read property 'name' of undefined
    at PrivateRouter.isActiveIntent (router_js.js:2158)
    at RouterState.isActiveIntent (router_state.js:19)
    at RoutingService.isActiveForRoute (routing.js:79)
    at Class._isActive (index.js:2845)
    at Class.computeLinkToComponentWillBeActive (index.js:2816)
    at index.js:2478
    at untrack (index.js:1018)
    at ComputedProperty.get (index.js:2477)
    at Class.CPGETTER_FUNCTION [as willBeActive] (index.js:638)
    at Class.computeLinkToComponentTransitioningIn (index.js:2854)

The ember app boots and renders fine. isActiveIntent is called, but seems to work correctly. When the request to the server is made, it returns 404, and the error handler is called. After transitionTo or intermediateTransitionTo is called, the next invocation of isActiveIntent fails with above error.

The issue is that recogHandlers now has a different length than targetRouteInfos

recogHandlers
0: {handler: "application", names: Array(0), shouldDecodes: Array(0)}
1: {handler: "public-version", names: Array(2), shouldDecodes: Array(2)}
2: {handler: "public-version.index", names: Array(0), shouldDecodes: Array(0)}
length: 3
targetRouteInfos
(2) [ResolvedRouteInfo, UnresolvedRouteInfoByParam]

0: ResolvedRouteInfo
  isResolved: true
  name: "application"
  paramNames: []

1: UnresolvedRouteInfoByParam
  isResolved: false
  name: "public-version"
  paramNames: (2)

When looking at tildeio/router.js one array (recogHandlers.length) is used to iterate through the other array (targetRouteInfos). As recogHandlers is larger than targetRoutesInfos, this will inevitably lead to exceptions, unless routeName matches one of the targetRouteInfos. In this case routeName is just index, which probably would not have matched, even if all three elements would have remained on targetRouteInfos.

    let index = 0;
    for (len = recogHandlers.length; index < len; ++index) {
      routeInfo = targetRouteInfos[index];
      if (routeInfo.name === routeName) {
        break;
      }
    }

I'll be happy to fix this issue if someone who is familiar with the currently implementation can provide a few pointers and explanations about why things are the way they currently are. It's just too much code and too less time to fully understand that by myself.

Even if this ends up being an ember issue, I think we should make router.js a little more resilient and if possible not depend on two arrays that need to stay in sync / or at least make sure they are in sync before assuming that they are.

st-h avatar Feb 19 '20 10:02 st-h

Workaround: Do never transition to a route that has a wildcard in its path. If you need to redirect to a not found route, you can split it up in two identical routes:

this.route('redirectable-not-found');
this.route('never-transition-to-this-not-found-route', { path: '/*' });

Not that great, but better than a broken app.

st-h avatar Feb 20 '20 16:02 st-h

This sounds familiar to https://github.com/emberjs/ember.js/issues/12945

kategengler avatar Mar 06 '20 19:03 kategengler

Workaround: Do never transition to a route that has a wildcard in its path. If you need to redirect to a not found route, you can split it up in two identical routes:

this.route('redirectable-not-found');
this.route('never-transition-to-this-not-found-route', { path: '/*' });

@st-h We've determined that this is the current workaround, and have added this to the agenda for our next face to face framework meeting.

MelSumner avatar Mar 06 '20 19:03 MelSumner

This sounds familiar to #12945

Yes, seems to be same issue. I am just a little surprised that this is so old, as I am pretty sure it hasn't been that long that I checked that my error routes would work correctly.

@MelSumner I think it would be helpful to determine if we should still try to fix the old logic, or if a rewrite would make more sense (as far as I have been told, there doesn't seem to be anyone who still knows how the current logic is supposed to work in detail)

st-h avatar Mar 07 '20 14:03 st-h