Problems using UseExceptionHandler() and UseStatusCodePagesWithReExecute()

[BillDines]

I am currently upgrading my Web Api from Asp.Net Core v1 with Versioning v1.0 to Asp.Net Core v2 and Versioning v2 (basically the latest stable versions of both). We are targeting .Net Framework, but its the same for .Net Core.

In our existing implementation we use the following in our startup class so we can add some content when errors occur:

         app.UseExceptionHandler("/api/v1.0/error");
         app.UseStatusCodePagesWithReExecute("/api/v1.0/error");

This is a simple error controller to go with it (not our actual one!):

   [Produces("application/json")]
   [Route("api/v{version:apiVersion}/error")]
   public class ErrorController : Controller
   {

      public IActionResult Index()
      {
         return Json(new { status = Response.StatusCode });
      }
   }

This worked fine in v1.0 of api versioning, however I am having problems now I have upgraded.

What happens is that the first time an exception occurs, or a 400+ status code is returned, the error controller is not called and an empty response containing the error code is returned to the client. On subsequent calls it works OK.

This is easily reproduced by adding the above code to the standard Web Api Asp.Net Core template in VS2017, adding Api versioning to the ValuesController and throwing exceptions or returning 400+ status codes from the controller methods:

   [ApiVersion("1.0")]
   [Route("api/v{version:apiVersion}/values")]
   public class ValuesController : Controller
   {
      // GET api/values
      [HttpGet]
      public IActionResult Get()
      {
         throw new Exception("Whoops!");
         //return BadRequest();
         //return Ok(new string[] { "value1", "value2" });
      }

      // GET api/values/5
      [HttpGet("{id}")]
      public string Get(int id)
      {
         return "value";
      }

      // POST api/values
      [HttpPost]
      public void Post([FromBody]string value)
      {
         throw new Exception("Whoops!");
      }

      // PUT api/values/5
      [HttpPut("{id}")]
      public void Put(int id, [FromBody]string value)
      {
      }

      // DELETE api/values/5
      [HttpDelete("{id}")]
      public void Delete(int id)
      {
      }
   }

Currently this problem is delaying my upgrade, so it would be great to get a fix or simple workaround!

thanks

Bill

[commonsensesoftware]

Using the error handling features for web APIs seems odd because clients usually expect to get 4xx-5xx with some type of structured error body that's in JSON or XML as opposed to HTML. Regardless, it's your decision.

This issue is happening due to a change in the action resolution process in 2.0, which fixed a bug that affected some, but not all, in 1.0. The ASP.NET Core design expects that the pipeline stops on the first action match. If there are no actions, the error path is taken, which is usually 404. This is combined by the fact that the platform might call the action selector multiple times. The consequence of this behavior is that API versioning cannot make its decision until all possible routes have been considered. The only way to know that is to have a catch all route that is executed at the end. Unfortunately, this causes some other end of route behaviors to behave unexpectedly as you are seeing. As you can imagine, it would be highly inefficient to re-evaluate the entire route tree on every request. To optimize things, API versioning tracks whether the versioning policy has been evaluated at least once for a given route. Once it has, it can make the final decision much earlier in the pipeline. This is what is resulting the behavior where you first see things not do what you expected and then subsequent calls are ok.

Most of the devs that have hit this issue were concerned about UI side of their applications. The workaround is described in the known limitations of ASP.NET Core routing. In your case, however, you want the behavior on the web API. This should still be possible, but you're likely going to have to register the routes handlers manually. I can't speak to all the other routing behaviors, but API versioning puts itself last and does it in such a way so that the order in which you register things (a la AddApiVersioning) is inconsequential. It seems you've provided a sufficient repro. I'll use it to try to put together a working example.

If you're really champing at the bit, consider looking at #194. There is a working sample project at the end that demonstrates how to configure the API versioning manually. The routing magic is the use of UseRouter. The IApiVersionRoutePolicy is a IRouter.

[BillDines]

Thanks for the quick response. I might take a look at the solution for 194 as you suggest. Either that or change the way we handle errors. Although after some research I still favour the route I have taken - we are returning a JSON body and the original response code from the error controller as you suggest in your opening comment. There doesn't seem to be a more reliable and easily maintainable way to do this where you know for sure you will catch all occurrences of 400+ status codes or unhandled exceptions whether they are returned from your application code or from within your middleware pipeline...

[commonsensesoftware]

Sounds reasonable. After thinking upon this issue some more, I realized there may be a few more options for you.

First, this issue should never affect the behavior of unhandled exceptions because the pipeline will be interrupted. This could also be handled with an ExceptionFilter. I believe you can even configure the error handler differently for UI and API-based routes (but I've never tried it).

If you're only interested in the error response hooks specific to API versioning (e.g. 400/405), there are options available. Granted, it's not the convenient one-stop shop, but there is a way. To change things, you just need to replace the default error response provider. The error responses topic will describe what you can expect to receive. Since the provider ultimately returns IActionResult, you can return pretty much whatever you want.

Give those topics a peek. It may be much easier and get you what you want compared to the alternatives.

[commonsensesoftware]

Were you able to come up with a workable solution?

[commonsensesoftware]

I'm still not entirely sure why you want to do this for an API, but this configuration yields error text correctly every time:

app.UseExceptionHandler(
    new ExceptionHandlerOptions
    {
        ExceptionHandler = async context =>
        {
            context.Response.ContentType = "text/plain";
            await context.Response.WriteAsync( "Welcome to the error page." );
        }
    } );
app.UseMvc();

That probably isn't exactly what you're looking for, but hopefully it points you in the right direction.

[BillDines]

In the end I have created some middleware to replicate the error behaviour I need. This middleware goes at the top of the pipeline to ensure it catches everything. Abbreviated code below:

public class ApiErrorHandlingMiddleware
   {
      private RequestDelegate next;

      public ApiErrorHandlingMiddleware(RequestDelegate next)
      {
         this.next = next;
      }

      public async Task Invoke(HttpContext context, ILogger<ApiErrorHandlingMiddleware> logger)
      {
         try
         {
            await next(context);
            if (context.Response.StatusCode >= 400)
            {
               await HandleExceptionAsync(context);
            }
         }
         catch (Exception ex)
         {
            logger.Log(LogLevel.Error, ex.ToString());
            await HandleExceptionAsync(context, true);
         }
      }

      private static Task HandleExceptionAsync(HttpContext context, bool isUnhandled = false)
      {
         if (context.Response.HasStarted)
         {
            //response already written earlier in the pipeline
            return Task.CompletedTask;
         }

         return context.Response.WriteAsync("whatever you need to return");

      }


   }

This allowed us to keep exception handling code defined in a single place without needing any breaking changes for our Api consumers. Its not quite as nice as the original solution IMO as it requires a bit more low level developer knowledge of how the middleware pipeline works rather than just creating a controller and letting Asp.Net worry about it, but its not really a big deal!

I guess this is still an outstanding issue as the error pages should work with Api versioning, but now I have an alternative its not a pressing issue for me.

Thanks for your help.

[commonsensesoftware]

Glad you got something working. I definitely don't like that these oddities result in a behaviors that developers don't expect. I support POLA. I'll continue to carry the sentiment forward with the ASP.NET team. To remove this from API versioning, there needs to be a way to retrieve all candidates up front and/or ensure that the IActionSelector is only called once or notify the IActionSelector that there won't be any more passes.

This behavior didn't exist in 1.0.* because it assumed that the action selection process only occurred once. That actually led to a different set of bugs. To resolve that issue, the design had to change in 1.1.*+ to capture all candidates and only make a decision at the very end. After at least one pass has been made for a route, the action selection process can short-circuit. That's why things may not behave correctly on the first pass, but then seem fine afterward.