Ember.js: Broken "link-to" helper prevents page from loading

[andhofmt]

The link generating helper, {{#link-to 'route' }} will prevent a page from loading if the route has been removed or rename.

For example, imagine that 'dashboard' is a valid page. And 'stats' was a valid route, but has recently been refactored and is now called 'statistics'. The following code will prevent 'dashboard' from loading even though 'dashboard' is a valid route:

Click {{#link-to 'stats'}}here{{/link-to}} to view stats.

Wouldn't it make more sense for the link-to helper to have soft-fallback support and default to a <a href=""></a> when the route is invalid and put an Ember.warn("Link-to helper points to invalid route"); in the console?

[mitchlloyd]

There are some trade-offs between failing early or attempting to make the error less harmful.

Personally I would rather get an earlier failure than have a link silently not working. This is because I don't think it's reasonable to have test coverage to click every link in your app and make sure it's working. But I do think it's reasonable to have test coverage that renders each page in your app.

The counter argument is that it is much worse to not render a page of your app than to have a dead link. However, because it is so easy to cause runtime errors in JavaScript apps it is too risky to not do _any_ testing of a page, even if that is just manually clicking through the app to make sure things are rendering.

A similar case in Ember is the use of the action helper. This helper will throw an error if an action can't be found in the current context. Again the strategy seems to be to fail as early as possible.

Here are some link-to behaviors given a route called routeName with one dynamic segment:

1. {{#link-to 'routeName' 1}}/route-name/1{{/link-to}}
2. {{#link-to 'routeName'}}/route-name/undefined{{/link-to}}
3. {{#link-to 'routeName' nothing}}/#{{/link-to}}
4. {{#link-to 'bogus'}}Error: There is no route named bogus{{/link-to}}
5. {{#link-to 'bogus' 1}}Error: There is no route named bogus{{/link-to}}
6. {{#link-to 'bogus' nothing}}/#{{/link-to}}
7. {{#link-to nothing 1}}/#{{/link-to}}

Looking at this behavior I find myself wishing for more early errors when possible. It seems like 2 might be able to throw an error if it knew the number of positionalParams it was given and the number of dynamic segments in the route (baring any wildcards). It seems strange that 6 does not throw an error. So strange that I would call that a bug.

In any event, there is no perfect solution here, but if I had to choose between silently creating a dead link or loudly throwing an Error I would choose the later for the apps that I work on.

[rwjblue]

Agreed, on all accounts. Erroring early is absolutely the right thing to do here.

Also, 6 above is definitely a bug (though a separate one from this one). Can you open an issue specifically for that?

[stefanpenner]

It seems like we want a softer failure in production, but still to report that there was an error for bug tracking code? But still an explicit explosion \w good context in development.

I suspect the more recent issues preventing initial renders from producing error pages in all cases where something explodes during render also hurts this.

This is a tad tricky actually, and we have several conflating issue which we may need to tease apart.

[mitchlloyd]

route-recognizer throws the relevant errors here: https://github.com/tildeio/route-recognizer/blob/de4d0fb5457d4552b701855e9b3f0220da4e9d18/lib/route-recognizer.js#L386

@stefanpenner, is there an established pattern for catching errors from libraries in production but not development? Is this something we would want to do or is there a better approach?

[krisselden]

The issue is any error thrown in a components hooks or cps that isn't caught there will leave the renderer in a bad state

[krisselden]

Currently I just destroy it so it stops looping

[andhofmt]

Particularly in production - soft fallback would be ideal. Fail fast in development and testing environments makes sense.

[pixelhandler]

@andhofmt @krisselden @mitchlloyd @stefanpenner is this still an issue, perhaps we should close or create a new reproduction of this, what do you think?

[pixelhandler]

Per our triage policy I'll close this out for now, feel free to reopen if the issue persists on the current release.

• https://github.com/emberjs/ember.js/blob/master/CONTRIBUTING.md#triaging-policy