9

I'm testing out an MVC 6 Web Api and wanted to implement logging into a global error handler. Just guaranteeing no errors get out of the system without being logged. I created an ExceptionFilterAttribute and added it globally in the startup:

public class AppExceptionFilterAttribute : ExceptionFilterAttribute
{
    public override void OnException(ExceptionContext context)
    {
        //Notice pulling from HttpContext Application Svcs -- don't like that
        var loggerFactory = (ILoggerFactory)context.HttpContext.ApplicationServices.GetService(typeof (ILoggerFactory));

        var logger = loggerFactory.Create("MyWeb.Web.Api");
        logger.WriteError(2, "Error Occurred", context.Exception);

        context.Result = new JsonResult(
            new
            {
                context.Exception.Message,
                context.Exception.StackTrace
            });
    }
}

Now in the startup, I'm adding this filter in:

services.Configure<MvcOptions>(options =>
{
    options.Filters.Add(new AppExceptionFilterAttribute());
});

This all seems kind of brute force...is there a better way to get here using MVC 6?

Things I don't like or am unsure about with this approach:

  1. Don't like pulling DI from http context
  2. Don't have much context about the controller that originated the error (perhaps I can get it from the context in some way).

The other option I can think of is having a base controller that accepts an ILoggerFactory that all controllers inherit from.

Was wondering if there was some kind of diagnostics middleware that would allow logging to be inserted...

4
  • You can use the error event in global.asax. What dont you like about your existing approach? Commented Mar 10, 2015 at 15:12
  • 1
    @RyanDansie: In MVC 6 there is a move away from Global.asax, so I figured there was a better way to handle this: stackoverflow.com/questions/24718640/… Commented Mar 10, 2015 at 15:24
  • I also added some things I don't like in there... Commented Mar 10, 2015 at 15:25
  • The official docs have some tips: docs.asp.net/en/latest/fundamentals/error-handling.html Commented Jun 9, 2016 at 3:55

3 Answers 3

11

You question has 2 parts. 1) DI injectable filters 2) Global error handling.

Regarding #1: You can use ServiceFilterAttribute for this purpose. Example:

//Modify your filter to be like this to get the logger factory DI injectable.
public class AppExceptionFilterAttribute : ExceptionFilterAttribute
{
    private readonly ILogger _logger;
    public AppExceptionFilterAttribute(ILoggerFactory loggerfactory)
    {
       _logger = loggerFactory.CreateLogger<AppExceptionFilterAttribute>();
    }
    public override void OnException(ExceptionContext context)
    {
        //...
    }
}

//Register your filter as a service (Note this filter need not be an attribute as such)
services.AddTransient<AppExceptionFilterAttribute>();

//On the controller/action where you want to apply this filter,
//decorate them like
[ServiceFilter(typeof(AppExceptionFilterAttribute))]
public class HomeController : Controller
{
....
}

You should be able to get the details of the controller from the ExceptionContext that is passed.

Regarding #2: From your previous post looks like you were playing with ExceptionHandlerMiddleware(source & extension source)...how about using that?...some info regarding it:

  • This middleware is generic and is applicable to any middleware which is registered after it and so any concepts like controller/action info is specific to MVC which that middleware wouldn't be aware of.
  • This middleware does not handle formatter write exceptions. You could write your own buffering middleware where you can modify the response body to be a buffered stream(MemoryStream) and let the MVC layer write the response to it. In the case of formatter write exceptions, you can catch it and send a 500 error response with details.
Sign up to request clarification or add additional context in comments.

3 Comments

Thanks again @Kiran, I think the ErrorHandlerMiddleware is the solution I was looking for. I actually didn't know that was out there.
I just ended up using the middleware linke above as a reference and created my own Middleware that logs the error only and then lets the exception bubble. So just let the 500 error flow through and get the standard output. I suppose if it's a 500 level error anyway, it's not something that can/should be easily parsed and retried.
What if I have a thread that runs outside owin? these exception handlers doesn't handle the exceptions raised inside that thread. I have read about an issue where it seemed to be a request about this but I'm not able to find the solution github.com/dotnet/corefx/issues/6398
1

An alternative way to perform global error handling is by using a ILoggerProvider.

The advantage to logging exceptions in this way is that it also captures errors which occur in places that an attribute would not catch. For example, exceptions that occur in Razor code could also be logged.

Here's a basic example with dependency injection:

Provider

public sealed class UnhandledExceptionLoggerProvider : ILoggerProvider
{
    private readonly IMyErrorRepository errorRepo;

    public UnhandledExceptionLoggerProvider(IMyErrorRepository errorRepo)
    {
        // inject whatever you need
        this.errorRepo = errorRepo;
    }

    public ILogger CreateLogger(string categoryName) =>
        new UnhandledExceptionLogger(errorRepo);

    public void Dispose()
    {
    }
}

Logger

public class UnhandledExceptionLogger : ILogger
{
    private readonly IMyErrorRepository errorRepo;

    public UnhandledExceptionLogger(IMyErrorRepository errorRepo)
    {
        this.errorRepo = errorRepo;
    }

    public IDisposable BeginScope<TState>(TState state) => 
        new NoOpDisposable();

    public bool IsEnabled(LogLevel logLevel) =>
        logLevel == LogLevel.Critical || logLevel == LogLevel.Error;

    public void Log<TState>(
        LogLevel logLevel,
        EventId eventId,
        TState state,
        Exception exception,
        Func<TState, Exception, string> formatter)
    {
        if (IsEnabled(logLevel))
        {
            errorRepo.LogError(exception);
        }
    }

    private sealed class NoOpDisposable : IDisposable
    {
        public void Dispose()
        {
        }
    }
}

Startup

public void ConfigureServices(IServiceCollection services)
{
    // Add framework services.
    services.AddMvc();
    services.AddTransient<IMyErrorRepository, MyErrorRepository>();
    services.AddTransient<UnhandledExceptionLoggerProvider>();
}

public void Configure(
    IApplicationBuilder app,
    IHostingEnvironment env,
    ILoggerFactory loggerFactory,
    UnhandledExceptionLoggerProvider provider)
{
    loggerFactory.AddProvider(provider);

    // ... all the rest of your startup code
}

Comments

0

I'm using ASP.NET Core, but this solution should work.

I have a Middleware that I created to log all requests that go through the pipeline. In their, I just wrapped it in a try catch so if it throws an exception, it logs to my database.

       public async Task Invoke(HttpContext context)
        {
            var sessionId = GetSessionId(context);
            var path = context.Request.Path;

            var startTime = DateTime.UtcNow;
            var watch = Stopwatch.StartNew();

            try
            {
                await _next.Invoke(context);
                watch.Stop();
            }
            catch (Exception exception)
            {
                watch.Stop();

                await _errorRepo.SaveException(exception, context.Connection.RemoteIpAddress.ToString(), sessionId);
            }
            finally
            {
#pragma warning disable 4014
                _requestLogRepo.LogRequest(
                    sessionId,
                    context.User.Identity.Name,
                    context.Connection.RemoteIpAddress.ToString(),
                    context.Request.Method,
                    path,
                    context.Request.ContentType,
                    context.Request.ContentLength,
                    startTime,
                    watch.ElapsedMilliseconds);
#pragma warning restore 4014
            }
        }

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.