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.

[sbsdevelop]

I have the requirement to return a JSON payload for each error / exception that is returned from a .net core 2.0 API. I thought UseExceptionHandler() and UseStatusCodePagesWithReExecute() would be the way to handle this but since I am also using APIVersioning I ran into this same issue. I have worked around it by adding my own middleware, but was curious if you had a better suggestion?
Thanks!

[commonsensesoftware]

UseStatusCodePagesWithReExecute() should not be applied to API routes (IMO as well as many others), but it's possible to make it work. UseExceptionHandler() just requires the correct configuration as shown above. Is that not working for you?

[sbsdevelop]

Shoot - I added this:

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

But I still got an empty payload / body - obviously I did something wrong :^(
In any case, is your point that an API route should not return any kind of body result (text, html, json etc...) or that only UseExceptionHandler() and UseStatusCodePagesWithReExecute() should not be used this way (I am assuming the former). I am also assuming that the developers requesting JSON format payloads for errors (in my case) just want consistency by always getting that payload whether there was missing/invalid data or a authentication error. Thanks for your thoughts!

[commonsensesoftware]

UseExceptionHandler() can definitely return a response. If it's not working, I suspect it could be a conflicting router or middleware. If it's not entirely clear, I can probably whip up a quick example.

UseStatusCodePagesWithReExecute() is typically used with UI pages. Instead of rendering 500 to a user in a browser, an alternate UI page is executed and shown. This can be configured by particular status code. APIs should not have this behavior. They either succeed or fail. Redirects are ok, but not for errors.

There is never a guarantee a server will respond with an error in a format the client understands. It's common for it to be application/json, but that still doesn't mean anything. In particular for errors, the response payload may not match the accept header. The client must know the media type and the expected payload shape. If the client knows both, great. Consistent error responses help mitigate this, but a good client makes a decision first off of the status code and, optionally, makes an additional decision based on the response body. Reverse proxies like IIS often return non-JSON error messages if it short-circuits the pipeline.

[sbsdevelop]

Thanks for the clarification :^)
Cheers!

[moraleslos]

@commonsensesoftware Encountering a similar issue with this thread. When you mentioned that UseStatusCodePagesWithReExecute() can "be configured by particular status code", how would you do that? I would like to use UseStatusCodePagesWithReExecute("/") for only 404 codes. All other codes should be returned as is. How would you do that with UseStatusCodePagesWithReExecute()?

[commonsensesoftware]

Again - I'm not sure why you'd ever do that for an API. Custom error pages are generally for UIs. An API client is usually not going to expect to get an HTML (application/html) response.

I haven't tried this out, but the following should work if added to your configuration:

// only map to status code pages for non-api requests
app.MapWhen(
 c => !c.Request.Path.StartsWithSegments( "api", StringComparison.OrdinalIgnoreCase ),
 a => a.UseStatusCodePagesWithReExecute( "error" ) );

You should be able to modify this further as appropriate for your needs.

[moraleslos]

Hi @commonsensesoftware,

So how would you do in this scenario-- I have an asp.net core webapi middle-tier and a reactjs client that uses react router which is completely client-side routing. The original reason why the app uses UseStatusCodePagesWithReExecute("/") is to re-route any server-side urls (they all produce 404s) back to the client-side root, where react router takes care of the rest. This works like a charm.

Now the app has implemented security and with that it can return a 401. Using UseStatusCodePagesWithReExecute("/") essentially masks that 401 into a 404.

So knowing my scenario, how would you handle the above in terms of error handling / status codes along with server-side -> client-side redirects? If there is a preferred solution, I would definitely like to know since I too believe what is implemented now is not exactly how it should be done.

[moraleslos]

@commonsensesoftware,

BTW that little code snippet doesn't work for me. It doesn't load the react app at all-- just a 404. Here's what I have:

app.MapWhen(
  c => !c.Request.Path.StartsWithSegments("/api", StringComparison.OrdinalIgnoreCase),
  a => a.UseStatusCodePagesWithReExecute("/")
);