elmah / Elmah

Error Logging Modules & Handlers for ASP.NET
https://elmah.github.io/
Apache License 2.0
306 stars 65 forks source link

Is it wrong to use DelegateHandlers for ApiController Binding Errors? #420

Open welldtr opened 7 years ago

welldtr commented 7 years ago

I was not able to log type binding errors into Elmah.

My ApiController method was:

        [HttpGet, Route("api/om/{NumOM}")]
        public OM ObterInfoOM(decimal NumOM)
        {
            ....
        }

And the wrong request was: http//{host}/api/om/notanumberstringthatfailsmymethod

So, the controller was not able to bind the parameters of my method (logically), giving the following error response (not an exception, but 400 response)

{
    "Message": "The request is invalid.",
    "MessageDetail": "The parameters dictionary contains a null entry for parameter 'NumOM' of non-nullable type 'System.Decimal' for method 'selene.scanpack.infrastructure.Pack`1[selene.scanpack.domain.Entity.OM] ObterInfoOM(System.Decimal)' in 'selene.scanpack.WebApp.Controllers.OMController'. An optional parameter must be a reference type, a nullable type, or be declared as an optional parameter."
}

But unfortunatelly elmah's webapi filter (ElmahRequestValidationErrorFilter) just catched "after binding" errors.

So I was using this implementation as workaround:

    /// <summary>
    /// MessageHandler to intercept WebApi calls that results in 400 status code and was not handled
    /// </summary>
    public class APIHandler : DelegatingHandler
    {
        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
        {
            try
            {
                return base.SendAsync(request, cancellationToken).ContinueWith((task) =>
                {
                    if (task.IsFaulted)
                    {
                        ErrorLog.GetDefault(HttpContext.Current).Log(new Error(task.Exception));
                    }
                    else if (task.Result.StatusCode == HttpStatusCode.BadRequest)
                    {
                        var error = task.Result.Content.ReadAsAsync<object>().Result as HttpError;

                        if (error != null)
                        {
                            HttpException exc = new HttpException((int)task.Result.StatusCode, error.MessageDetail);
                            ErrorLog.GetDefault(HttpContext.Current).Log(new Error(exc));
                        }
                    }

                    return task.Result;
                });

            }
            catch (Exception e)
            {
                TaskCompletionSource<HttpResponseMessage> tsc = new TaskCompletionSource<HttpResponseMessage>();
                ErrorLog.GetDefault(HttpContext.Current).Log(new Error(e));
                return tsc.Task;
            }
        }
    }

Is this implementation right?