Custom IOutputFormatter not executed when using conventional routing

maurei commented 4 years ago

Description

I'm using conventional routing: I'm programmatically assigning templates to controllers, ie not using [ApiController][Route(" .. ") but an IApplicationModelConvention implementation. Also, I have a custom IOutputFormatter. The bug is that my custom formatter is not always executed: it depends on how my routing is configured. Below is my controller in which I use conventional routing

// controller
public class WeatherForecastConventionRoutingController : ControllerBase
{
    [HttpGet] public object Get() => Ok("foobar");
    [HttpGet("foos")] public object GetFoos() => NotFound("foobar");
    [HttpGet("bars")] public object GetBars() => NotFound();
}

When triggering the first and second actions my formatter is executed. For the last action my custom formatter is entirely skipped.

This behaviour is different when using attribute based routing. In this case, for all three actions below my custom outputter is executed:

[ApiController]
[Route("[controller]"))]
public class WeatherForecastAttributeRoutingController : ControllerBase
{
    [HttpGet] public object Get() => Ok("foobar");
    [HttpGet("foos")] public object GetFoos() => NotFound("foobar");
    [HttpGet("bars")] public object GetBars() => NotFound();
  }

This behaviour being different is unexpected and I believe it is a bug.

To Reproduce

Clone this repro repository. Run it and check out and hit the endpoints


/weather-forecast-convention           // => "barfoo"
/weather-forecast-convention/foos      // => "barfoo"
/weather-forecast-convention/bars      // => bug: no content

/weather-forecast-attribute               // => "barfoo"
/weather-forecast-attribute/bars      // => "barfoo"
/weather-forecast-attribute/foos      // => "barfoo"

All of them should return the same output but this is not the case.

Use-case

In case the reader is wondering why this is relevant: I'm working on a framework that implements json:api. We're using conventional routing because we want to programmatically configure routing for better extensibility and at the same time we want to use a custom formatter to add additional error details when an entity is not found.

The work-around for now is to return NotFound(null) instead of NotFound().

Further technical details

.NET Core SDK (reflecting any global.json):
 Version:   3.0.100
 Commit:    04339c3a26

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  10.14
 OS Platform: Darwin
 RID:         osx.10.14-x64
 Base Path:   /usr/local/share/dotnet/sdk/3.0.100/

Host (useful for support):
  Version: 3.0.0
  Commit:  7d57652f33

.NET Core SDKs installed:
  2.1.700 [/usr/local/share/dotnet/sdk]
  2.2.301 [/usr/local/share/dotnet/sdk]
  3.0.100 [/usr/local/share/dotnet/sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.11 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.6 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.11 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.6 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.11 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.13 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.6 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

javiercn commented 4 years ago

@maurei thanks for contacting us.

@pranavkm Can you tell what's going on here/what's missing?

pranavkm commented 4 years ago

Do you have an [ApiController] annotation in the first case? If not, that might explain the difference - controller instances annotated with ApiController attribute transform status code results in to ObjectResults with ProblemDetails - https://docs.microsoft.com/en-us/aspnet/core/web-api/?view=aspnetcore-3.0#problem-details-for-error-status-codes. In the absence of the ApiController annotation, all you have is a regular status code result which does not need to be formatted.

We're using conventional routing because we want to programmatically configure routing for better extensibility and at the same time we want to use a custom formatter to add additional error details when an entity is not found.

Why not add a result filter that transforms NotFoundResult \ NotFoundObjectResult to return a custom result type?

maurei commented 4 years ago

Thanks for your time guys, greatly appreciated

Yes the lack of [ApiController] is one case is the difference, see the repro repository.

all you have is a regular status code result which does not need to be formatted.

I.e. the formatters not being executed is by design? That troubles me a bit from an intuitive point of view.

In the typical usage of the boilerplate project there might not be any content to format, but that shouldn't necessarily mean that there is no need to add content and format. And I feel like the result filter isn't the place to do that?
In the current design the developer needs to know how .net core internally handles return NotFound() versus return NotFound(content) in the presence or absence of [ApiController]. (Is there documentation about this?) I feel like switching from attribute routing to conventional routing shouldn't affect the middleware any other than what is necessary for routing

Why not add a result filter that transforms NotFoundResult \ NotFoundObjectResult to return a custom result type?

I guess I could. But is this the intended way to deal with this? My concerns with this are:

The custom errors that we add are shaped in the same format as our regular API content (which is the json:api format), i.e. the same serializer is used. I feel it shouldn't be necessary to have to inject his custom serializer in another place than the formatter (or multiple places in general) to give shape to result content
The result filters aren't executed in the case of exceptions or short-circuit because of authorization. That means, afaik, in my case I would only have to use a filter like this because I switched away from using [ApiController]

Thanks again for your time 👍

wisepotato commented 4 years ago

Any update on this?

pranavkm commented 4 years ago

.e. the formatters not being executed is by design?

Yes, formatters are only invoked when there are objects to serialize. There are plenty of action result types, including StatusCodeResult, FileStreamResult, ViewResult, ChallengeResult etc, that do not incur a formatter being invoked.

The result filters aren't executed in the case of exceptions or short-circuit because of authorization

You should use an IAlwaysRunResultFilter.

ghost commented 4 years ago

Thank you for contacting us. Due to no activity on this issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

dotnet / aspnetcore