5

While trying to find the best approach to implementing a catch-all for any uncaught exceptions I found this.

But, while implementing it, I remembered where I read:

WARNING Don't modify the Response object after calling next()..., as the response may have already started sending and you could cause invalid data to be sent.

pg. 580

Is this an issue when the middleware is acting as a global exception handler before the MVC middleware where it seems reasonable that if the exception middleware were invoked, no response could have been started?

Invoke on Middleware:

public async Task Invoke(HttpContext context)
{
    try
    {
        await _next(context);
    }
    // A catch-all for uncaught exceptions...
    catch (Exception exception)
    {
        var response = context.Response;
        response.ContentType = "application/json";
        response.StatusCode = (int)HttpStatusCode.InternalServerError;

        await response.WriteAsync(...);
    }
}
ChiefTwoPencils
  • 13,548
  • 8
  • 49
  • 75
  • 1
    If “next” failed with exception. Means there was not response sending yet, and you can catch and send whatever you want. Make sense? – grinay Aug 13 '19 at 04:44
  • 1
    Also, read this article https://learn.microsoft.com/en-us/aspnet/core/fundamentals/middleware/?view=aspnetcore-2.2. You can always check if the response has been already sent by checking property HasStarted on HttpResponse object. – grinay Aug 13 '19 at 05:33
  • Why don't you use [`UseExceptionHandler`](https://learn.microsoft.com/en-us/aspnet/core/fundamentals/error-handling?view=aspnetcore-2.2#exception-handler-lambda)? You can access the exception, as shown here: https://stackoverflow.com/a/55166404/2698119 – Métoule Aug 13 '19 at 07:31
  • @Métoule, I'm not a fan of the implementation. I'd prefer not bloat my Startup with the handling code but more importantly it's useful as a reusable middleware for other projects. – ChiefTwoPencils Aug 13 '19 at 15:58

1 Answers1

1

Of course, I cannot comment on the exact intention the author had in mind when writing that. But the general idea is that you shouldn’t modify a HTTP response when it has already started. This is because the response can already be sent in parts before it is completely constructed. And when you then attempt to change the request then, you will get exceptions that you cannot modify the response.

That is why when you invoke some middleware, and that middleware is expected to produce a response, then you should not modify the response; simply because it will likely fail.

If you invoke some middleware, and that middleware does not produce a response, then of course you are free to still create a response.

For exceptions in particular, middlewares usually produce the response at the very last step, e.g. MVC works with action result objects internally and only at the end those will be executed to produce an actual result on the response. So exceptions will often be triggered before a response is produced. So it is fine to modify the response if you encounter an exception.

The built-in exception handler middleware works pretty much the same btw., so that should be a sign that what you are doing is fine. You should just be aware that modifying the response could potentially fail, so you should handle that case as well, e.g. by checking the HasStarted property.

poke
  • 369,085
  • 72
  • 557
  • 602
  • Thanks, my main concern was hiding something important and how to handle it. When the try is added directly to the controller for example, I'm confident were the exception was thrown and certain the response has not started. But once I'm outside the pipeline it's no longer obvious or guaranteed. After looking at the ExceptionHandler, I may provide a convenience wrapper around that to make use of their superior implementation and get over my gripes. – ChiefTwoPencils Aug 13 '19 at 21:25