[Discussion] Async suffix for controller action names will be trimmed by default

[pranavkm]

As part of addressing #4849, ASP.NET Core MVC will trim the suffix Async from action names by default. This affects routing and link generation.

Consider

public class ProductController : Controller
{
    public async Task<IActionResult> ListAsync()
    {
        var model = await DbContext.Products.ToListAsync();
        return View(model);
    }
}

Prior to 3.0, the action will be routeable via Product/ListAsync. Link generation would require specifying the Async suffix e.g.

<a asp-controller="Product" asp-action="ListAsync">List</a>

In 3.0, the action will be routeable via Product/List and link generation would require not specifying the Async suffix e.g.

<a asp-controller="Product" asp-action="List">List</a>

This change does not affect names specified using the ActionNameAttribute. This behavior can be disabled by setting MvcOptions.SuppressAsyncSuffixInActionNames to false as part of the application startup:

services.AddMvc(options =>
{
   options.SuppressAsyncSuffixInActionNames = false; 
});

[sgjsakura]

Tip: In your code sample you missed the Task wrapper for return type.

Maybe you should also update the code sample in the reference.

Best wishes~

[manigandham]

Why? Adding magic to change names seems like a source of bugs. There's no reason to use the async suffix in the first place for all of the actions.

[poke]

@manigandham See the linked issue for details. The code fix to make a method asynchronous adds the Async suffix to the method name.

Note that there's not really any magic going on. The generated action name will just omit “Async” at the end. And remember that the framework already does something similar for controller names.

[grprakash]

Why? Adding magic to change names seems like a source of bugs. There's no reason to use the async suffix in the first place for all of the actions.

Meaningful conventions are not magic. Its been very useful in situations like "Controller", "Attribute" etc

[Genbox]

In case there is an Index and IndexAsync action, which one will take precedence?

[manigandham]

Wouldn't it be better to update the code fix itself? The async suffix convention seems outdated since async is the majority and actions aren't called by users directly anyway.

[Morcatko]

I agree with @manigandham. I don't believe .NET (or any framework in general) should be driven by "bugs" in its code fixes, but the other way.
Another thing - this fixes code fix vs route issue, but breaks case when someone uses nameof(method) (for whatever reason)

[BlueMarmalade]

what if you have two methods with the same names; one is async and one is not? Isn't it strange to have to rename the synchronous method to something completely different? (But maybe i'm just not getting it ;P)

[poke]

@BlueMarmalade If you already have two separate actions with the same name but one is ~Async, then I would highly rethink your design because that seems to be a rather bad idea to begin with.

As for what happens if you have both, I would assume that they both take the same route and as such the first action will win.

[TsengSR]

@poke

As for what happens if you have both, I would assume that they both take the same route and as such the first action will win.

I'd expect an AmbiguousActionException, which we got before when two actions did match the same route, i.e. two get methods which only differ in query parameters

@BlueMarmalade

what if you have two methods with the same names; one is async and one is not? Isn't it strange to have to rename the synchronous method to something completely different? (But maybe i'm just not getting it ;P)
I assume you can still do that, by explicitly adding an

    [HttpGet("IndexAsync")]
    public async Task<IActionResult> IndexAsync() { .. }

    [HttpGet]
    public async Task<IActionResult> Index() { .. }

to have it explictly. As of today, you had to do the exact opposite to reach the goal

    [HttpGet("index")]
    public async Task<IActionResult> IndexAsync() { .. }

which is counter intuitive since most actions will be async.

[terrajobst]

Wouldn't it be better to update the code fix itself? The async suffix convention seems outdated since async is the majority and actions aren't called by users directly anyway.

No. In fact, we're talking about adding more sync overloads for APIs that were 100% async before. Async is viral and not all code can be async for practical reasons.

On top, even if new tech could be a 100% async, the overwhelming majority of APIs and techologies are sync. It's an important part of the developer experience to have consistency. We found that consistency across multiple technologies is way more important than a local optimum. The reason being that most developers have to write code in various areas and not having to deal with different (or even conflicting) guidance is important.

I agree with the issue opener: MVC should ignore the Async suffix.

[NickCraver]

FWIW, while I agree with the behavior here is likely the wanted one most of the time, I just hit in in practice on some sample apps that had MyAction and MyActionAsync. If this was compile-time or start-up time that'd be one thing...but I'm only seeing errors only at runtime. For example:

Samples.AspNetCore.Controllers.TestController.DuplicatedQueriesAsync (Samples.AspNetCore3)
Samples.AspNetCore.Controllers.TestController.DuplicatedQueries (Samples.AspNetCore3)
   at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.ReportAmbiguity(CandidateState[] candidateState)
   at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.ProcessFinalCandidates(HttpContext httpContext, CandidateState[] candidateState)
   at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.Select(HttpContext httpContext, CandidateState[] candidateState)
   at Microsoft.AspNetCore.Routing.Matching.DfaMatcher.MatchAsync(HttpContext httpContext)
   at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context)

Can we improve this 2-matching-routes scenario (which worked before and now breaks) by alerting the user in some way before having to specifically hit one of the duplicate routes at runtime and debug what's breaking?

[NickCraver]

In addition to the duplicate scenario above, this simple case breaks:

<a asp-controller="Test" asp-action="MyActionAsync">Link Text</a>

public async Task<IActionResult> MyActionAsync() { }

Why should this break? It's only being handled on one side of the fence. And again, users will only find this out at runtime and only if they go test every route.

[pflannery]

services.AddMvc(options => { options.SuppressAsyncSuffixInActionNames = false; });

Having this option wouldn't hurt I just don't think it should be imposed out of the box.
I think this would be better as an opt-in flag like options.RemoveAsyncSuffixFromActionNames = true.

The original issue is easily resolved by changing return View() to return View("Index").
Either that or OP can drop the suffix "Async" from the method name all together.

People should be left to their own naming conventions without interruption.

Strongly typed areas, controllers and views

There are similar issues that have been around a while that I think also need addressing with the UrlHelper and ControllerBase.

• Trim the "Controller" suffix from the controller name so we can use the nameof operator.
  i.e. UrlHelper.Url(nameof(SomeController.ActionName), nameof(SomeController))
• Ability to determine area routing using the controller AreaAttribute.
  i.e. UrlHelper.Url<SomeController>(nameof(SomeController.ActionName))

Also view resolution:

• Have the option to generate View class names without the "folder location" prefixes to allow us to use nameof for strongly typed resolution. i.e. return View(nameof(Views.Index))

[terrajobst]

People should be left to their own naming conventions without interruption.

I disagree. The default path ("pit of success") should generate in solutions that follow the standard .NET naming conventions. Of course people should be able to enable whatever naming conventions they want. But I assert that following the common conventions shouldn't require configuration changes because we shouldn't penalize folks for doing the right thing. A successful framework cannot be neutral w.r.t naming conventions. The value is in consistency.

[wagich]

As @pflannery mentions, this turns out to be a massive headache when using nameof() to "strongly" type action and controller names, which I (used) to do everywhere.
It's much nicer being able to refactor controllers and actions without having to use a bunch of consts or even shudder sprinkling strings everywhere…

I'm very much not opposed to the new behavior, but it really should do the same transformation on passed-in names when constructing a path/url.

[terrajobst]

@wagich

I'm very much not opposed to the new behavior, but it really should do the same transformation on passed-in names when constructing a path/url.

That one I agree with.

[artkrv]

My teammate waitsted entire day on migration 2.2 --> 3.1 because of this issue.

[BlueMarmalade]

i still don't see the point of this change. i'm sure alot of apps have hard coded paths to certain controller methods. changing it like this is just one of those changes that tries to make things easier by hiding/removing stuff by default, but it has little purpose and actually makes things more confusing if anything.

[Cammeritz]

coded paths to certain controller methods.

Yeah, we wasted two days finding out what the issue was while trying to fix the problem in 100 different ways. We multiple Applications that listen to those paths and now we had to change them all.

Also, this did not work for us:

services.AddMvc(options =>
{
   options.SuppressAsyncSuffixInActionNames = false; 
});

Great move Microsoft 👏

[pranavkm]

Is it possible to "fix" UrlHelper / LinkGenerator to trim "Controller" and "Async" suffixes too?

I think that's what we vaguely decided on part of #14103.

[ghost]

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[deeprobin]

Unfortunately, this still does not work in ASP.NET 5.0.2... Gladly assign me, then I will create a pull request as soon as I get to it.

[sfsharapov]

Well, it doesn't in 6th LTS version too. It's so disgusting, this bug is undocumented being so obvious to developers. Yesterday I've spent all the day because of this bug.

[deeprobin]

My first suggestion would be that we truncate the async suffix respecting the property SuppressAsyncSuffixInActionNames in the UrlHelperExtensions (see example).

Are there any better options?

---

Also, I think we should put this issue on the .NET 7.0.0 milestone, as this is imo a very important feature for many users.

[ADH-LukeBollam]

It is quite strange to find the name of my method has been modified. I'm using reflection to read Action argument types in some middleware with var action = _httpContextAccessor.HttpContext.Request.RouteValues["action"].ToString(); and I was very surprised to see Async had been removed from my Action name.

It seems like a change that has good intentions, but is another gotcha that a dev has to keep in mind. I personally don't like my runtime not reflecting the code that I write.

[Luk164]

@LukeBolly At least give us a compiler warning or something right? When it happened to me it wasn't even mentioned in the main documentation, just in a migration-guide.

[wagich]

It would be extremely welcome if at least the link-generation could be fixed to do the same thing.
Because right now one half of the routing infra removes the async-suffix and the other half doesn't - which just opens a big old hole for anyone to fall into.

[Luk164]

a big old hole for anyone to fall into.

@wagich I feel called out. But I do agree with the sentiment. If it got cut on both sides it makes sure that the code at least works. Still feels like putting some planks over the hole though.

[wagich]

@Luk164 That was not my intention!
Speaking for myself, it took me the better part of a day (and debugging into the framework code) to figure out why my urls generated using nameof all were suddenly blank.

[devel0]

The change of SuppressAsyncSuffixInActionNames default value to false would be a breaking change respect to current asp net core 7.

The reasons leading to this decision regarding the default value are not explained, btw I suppose the reason is related to an overlap of:

• better practice to append Async to async method names
• short name on web api url and because sync method on web api call doesn't make sense

To avoid errors I updated my best practice about async into the following:

append "Async" to async methods except those related to webapi mapping in ControllerBase specializations

example:

[ApiController]
[Authorize]
[Route("api/[controller]/[action]")]
public class AuthController : ControllerBase
{
    private readonly IAuthService authService;

    public AuthController(IAuthService authService)
    {
        this.authService = authService;
    }

    [HttpPost]
    [AllowAnonymous]
    public async Task<ActionResult<LoginDto>> Login([FromBody] LoginRequestDto login) =>
        this.CommonResponse(await authService.LoginAsync(login));
}