josephwoodward / GlobalExceptionHandlerDotNet

Exception handling as a convention in the ASP.NET Core request pipeline
MIT License
269 stars 32 forks source link

OnError endpoint and exception map #39

Closed Kromi75 closed 4 years ago

Kromi75 commented 4 years ago

I'm using the OnError endpoint to write exceptions to serilog. Additionally I have mapped ValidationException to status code 400. In order to test my GlobalExeptionHandler configuration I have distributed some new ApplicationException("...") in my code. But these never get logged. Only the ValidationExceptions are. If I add a map for ApplicationException, these get logged, too. Is this the intended behavior?

josephwoodward commented 4 years ago

Yes, if I'm understanding you correctly then this is the expected behaviour. If an exception isn't mapped then the response GlobalExceptionHandler will return to callers is the default response.

Take the following example:

public void Configure(IApplicationBuilder app, IHostingEnvironment env)
{
app.UseGlobalExceptionHandler(x => {
    x.ContentType = "application/json";
   // This is a default error response
    x.ResponseBody(s => JsonConvert.SerializeObject(new
    {
        Message = "An error occurred whilst processing your request"
    }));

    x.Map<ValidationException>().ToStatusCode(StatusCodes.Status400BadRequest);
});

     // Returns 400 plus whatever is in default response ResponseBody (eg: "An error occurred whilst processing your request")
    app.Map("/validationerror", x => x.Run(y => throw new ValidationException()));

   // returns whatever is configured in the default response ResponseBody (eg: "An error occurred whilst processing your request")
    app.Map("/applicationerror", x => x.Run(y => throw new ApplicationException()));
}

I hope this helps, let me know if you have any further problems or questions.

Kromi75 commented 4 years ago

I'm not sure if we talk about the same thing. Please have a look at this snippet:

      app.UseGlobalExceptionHandler(
        x =>
        {
          x.ContentType = "application/json";
          x.ResponseBody(s => JsonConvert.SerializeObject(new { Message = "An error occurred whilst processing your request" }));

          x.OnError((exception, httpContext) =>
          {
            // This is not executed for exceptions of type ApplicationException.
            Log.Error("An error occurred. {Exception}", exception);
            return Task.CompletedTask;
          });

          // Validation-Exceptions are to be returned as Bad Request.
          x.Map<ValidationException>().ToStatusCode(StatusCodes.Status400BadRequest);
        },
        NullLoggerFactory.Instance);

With this configuration, the OnError func is called for exceptions of type ValidationException, but not for ApplicationException. So I assume that the OnError func is only called for exception types a mapping is defined for. Am I right? Or am I doing it completely wrong? BTW, what I'm trying to achieve is that all exceptions are logged exactly once. That's why I passed a NullLoggerFactory to avoid having all exceptions logged as unhandled exceptions.

josephwoodward commented 4 years ago

This should be fixed in the v5 release which will be going out in the next day or so.

Kromi75 commented 4 years ago

Great, thank you.