Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Reconsider routing behavior where URL generation silently fails when route would point to invalid action #302

Closed
Eilon opened this issue Apr 24, 2014 · 9 comments

Comments

@Eilon
Copy link
Member

Eilon commented Apr 24, 2014

Scenario: I'm building an app one action at a time. I've set up some generic routes so far. My pattern is that I code up an action, then build its corresponding view page, and then in that view page I call helpers such as Html.ActionLink and Url.Action to generate links to the next few actions I plan to work on.

The problem I encountered is that none of those links got generated! As it turns out, after I got reminded by @rynowak, is that we don't generate links to actions that don't yet exist. Nor do we tell the developer of this problem.

In MVC the behavior was to blindly generate links: I think that's actually what I want to happen here. I know exactly what I'm doing and I want to have the broken links while I'm developing my app. While building an app I already know that a lot of stuff doesn't work - I'm still in the middle of building it.

An alternative that I personally find less appealing is to throw an exception if the generated link will point to an action that doesn't exist.

I could imagine a "diagnostic mode" of some sort that I could enable after I finished developing my app. But while I'm working on my app I think it would get in the way of my flow.

@yishaigalatzer yishaigalatzer added this to the Post Alpha milestone Apr 30, 2014
@yishaigalatzer yishaigalatzer removed this from the Post Alpha milestone May 23, 2014
@javiercn
Copy link
Member

@harshgMSFT Ran into this exact scenario with Html.BeginForm, I think failing silently is a bad idea.

I'm more inclined with the approach of throwing rather than generating broken links. The reason is because when I'm developing, if for example I misspell a letter from any of the values I still generate a link that I might think works (because I generated a link) but is not valid, rather than if I throw I get feedback instantly and get pointed on the right direction).

However, I also understand that it'll be frustrating to throw just because you didn't wrote a controller or action for a link. I follow a similar flow as the one that you described when I develop MVC apps and I would find throwing every time very annoying.

In the end I believe there is no silver bullet for this so I think a third option could be to make this behavior configurable. (You might turn it off when you are developing (if you choose to do so)) and enforce it at runtime when you run tests, etc. That option is best of both worlds I think.

@xqiu
Copy link

xqiu commented Sep 17, 2014

For BeginForm razor generation in #1148, it's actually a bug without showing the correct link in the generated HTML even I don't have a controller yet. Because when I met this behavior, I didn't think it's because I don't have a controller, but think there are something wrong with my code that didn't give the correct ActionName.

@danroth27 danroth27 added this to the 6.0.0-rc1 milestone Oct 16, 2014
@danroth27 danroth27 modified the milestones: 6.0.0-rc1, 6.0.0-beta2 Oct 20, 2014
@yishaigalatzer yishaigalatzer modified the milestones: 6.0.0-rc1, 6.0.0-beta2 Dec 12, 2014
@rynowak
Copy link
Member

rynowak commented Dec 12, 2014

@yishaigalatzer is there a consensus here? or something we want to design for RC.

@yishaigalatzer
Copy link
Contributor

No further discussion was made. We need to reach a design and execute for rc

@yishaigalatzer yishaigalatzer modified the milestones: 6.0.0-rc1, 6.0.0-beta3 Jan 14, 2015
@yishaigalatzer
Copy link
Contributor

Related to #1097

@rynowak
Copy link
Member

rynowak commented Feb 3, 2015

We have competing desires in link generation:

Sometimes a user wants to rapidly prototype and generate links to actions that don’t yet exist. If link generation throws or returns bad data this really messes up your flow.

Sometimes you want to know if link generation fails – you probably have a bug in your code. Silently failing to generate a link, or silently generating one that we can prove will 404 is bad.

These desires are opposites, for prototyping you want to get as correct of an answer as possible, and for general maintenance you want to be able to find out as quickly as possible that you have a problem.

Decision: We want to make this behavior configurable in MvcOptions. Default setting will be to only generate links to valid actions.

Suggestion: We might want to add some code to the template that shows how to tweak this, or enable it in the dev environment in the template.

But wait, there’s more

MVC 6 link generation is smarter than previous versions. Link generation has fallback just like URL matching. After we generate a URL, we validate its value to ensure that it would reach a valid action if round-tripped. This allows us to avoid some ordering problems with conventional routes. If we just stop validating the route values to implement this feature than the behavior is inconsistent in subtle ways.

Decision: We’ll pass through the option described above into routing on the VirtualPathContext. If we fail to validate the route values for all routes, then we’ll fall back to the first route with URL but invalid values. This is consistent with the current behavior for successful link generation and provides a best guess in the prototyping scenario.

rynowak added a commit to aspnet/Routing that referenced this issue Feb 10, 2015
This change adds a feature needed for aspnet/Mvc#302

There's a new option in routing that allows link-generation to proceed
when the route values cannot be validated. The key scenario for this is
during development of an MVC site. Routing will refuse to generate a link
to actions which don't exists, this is a breaking change from the MVC5
behavior. Setting UseBestEffortLinkGeneration will allow routing to return
a value even when we can't match the action.

This option will remain off by default - setting this to on will impact
link-generation in a bunch of scenarios involving areas where we've
improved the logic for MVC6. If you're considering leaving this on outside
of development scenarios, then make sure to be as explicit with route values
as possible (don't rely on ambient values).

Functional tests to follow up in the MVC repo.
@rynowak
Copy link
Member

rynowak commented Feb 10, 2015

aspnet/Routing#150

rynowak added a commit to aspnet/Routing that referenced this issue Feb 10, 2015
This change adds a feature needed for aspnet/Mvc#302

There's a new option in routing that allows link-generation to proceed
when the route values cannot be validated. The key scenario for this is
during development of an MVC site. Routing will refuse to generate a link
to actions which don't exists, this is a breaking change from the MVC5
behavior. Setting UseBestEffortLinkGeneration will allow routing to return
a value even when we can't match the action.

This option will remain off by default - setting this to on will impact
link-generation in a bunch of scenarios involving areas where we've
improved the logic for MVC6. If you're considering leaving this on outside
of development scenarios, then make sure to be as explicit with route values
as possible (don't rely on ambient values).

Functional tests to follow up in the MVC repo.
rynowak added a commit to aspnet/Routing that referenced this issue Feb 10, 2015
This change adds a feature needed for aspnet/Mvc#302

There's a new option in routing that allows link-generation to proceed
when the route values cannot be validated. The key scenario for this is
during development of an MVC site. Routing will refuse to generate a link
to actions which don't exists, this is a breaking change from the MVC5
behavior. Setting UseBestEffortLinkGeneration will allow routing to return
a value even when we can't match the action.

This option will remain off by default - setting this to on will impact
link-generation in a bunch of scenarios involving areas where we've
improved the logic for MVC6. If you're considering leaving this on outside
of development scenarios, then make sure to be as explicit with route values
as possible (don't rely on ambient values).

Functional tests to follow up in the MVC repo.
rynowak added a commit to aspnet/Routing that referenced this issue Feb 10, 2015
This change adds a feature needed for aspnet/Mvc#302

There's a new option in routing that allows link-generation to proceed
when the route values cannot be validated. The key scenario for this is
during development of an MVC site. Routing will refuse to generate a link
to actions which don't exists, this is a breaking change from the MVC5
behavior. Setting UseBestEffortLinkGeneration will allow routing to return
a value even when we can't match the action.

This option will remain off by default - setting this to on will impact
link-generation in a bunch of scenarios involving areas where we've
improved the logic for MVC6. If you're considering leaving this on outside
of development scenarios, then make sure to be as explicit with route values
as possible (don't rely on ambient values).

Functional tests to follow up in the MVC repo.
rynowak added a commit that referenced this issue Feb 10, 2015
This feature allows routing to generate a link when the action that is
being linked-to does not yet exist. See the PR in routing for the actual
implementation changes. This PR just has a simple functional test for the
scenario.
rynowak added a commit to aspnet/Routing that referenced this issue Feb 11, 2015
This change adds a feature needed for aspnet/Mvc#302

There's a new option in routing that allows link-generation to proceed
when the route values cannot be validated. The key scenario for this is
during development of an MVC site. Routing will refuse to generate a link
to actions which don't exists, this is a breaking change from the MVC5
behavior. Setting UseBestEffortLinkGeneration will allow routing to return
a value even when we can't match the action.

This option will remain off by default - setting this to on will impact
link-generation in a bunch of scenarios involving areas where we've
improved the logic for MVC6. If you're considering leaving this on outside
of development scenarios, then make sure to be as explicit with route values
as possible (don't rely on ambient values).

Functional tests to follow up in the MVC repo.
@rynowak
Copy link
Member

rynowak commented Feb 11, 2015

Fixed by aspnet/Routing 9ee946073a128f778cfc68fa796b95dc9c8e429a

@Eilon
Copy link
Member Author

Eilon commented Feb 11, 2015

🎆

rynowak added a commit that referenced this issue Feb 17, 2015
This feature allows routing to generate a link when the action that is
being linked-to does not yet exist. See the PR in routing for the actual
implementation changes. This PR just has a simple functional test for the
scenario.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants