Search code examples
c#asp.netasp.net-coreaction-filter

Catching errors from services in Exception Filter


I've created a service that's used throughout my aspnet project that retrieves and validates a header among other things. Issue is that the Exception Filter is not able to catch the errors that are thrown by the service as it's not in the scope of the Exception Filter thus giving the user an ugly internal server error. Is there any way to gracefully return an argument error with description to the user with the use of the services?

The Startup:

services.AddScoped<UserService>();
services
    .AddMvc(x =>
    {
        x.Filters.Add(typeof(Filters.MyExceptionFilter));
    })

The Exception Filter:

public class MyExceptionFilter : IExceptionFilter
{
    public void OnException(ExceptionContext context)
    {
        if (context.Exception is ArgumentException argumentException)
        {
            var response = context.HttpContext.Response;
            context.ExceptionHandled = true;
            response.StatusCode = 400;
            context.Result = new ObjectResult(argumentException.Message);
            return;
        }
    }
}

The Service:

public class UserService
{
    public readonly string UserId;

    public UserService(IHttpContextAccessor context)
    {
        if (!context.HttpContext.Request.Headers.TryGetValue("x-test", out var user))
        {
            throw new ArgumentException($"x-test header is required.");
        }
        UserId = user;
        //do other stuff
    }
}

Controller Action Method:

public async Task<IActionResult> Delete(string id, 
    [FromServices] UserService userService)
{
    //do stuff
}

Solution

  • A rule-of-thumb in C# is do as little work in the constructor as possible. There's a few good reasons for this (e.g. you can't dispose a class that threw an exception in it's constructor). Another good reason is, as you have found out, construction might happen in a different place (e.g. a DI container) than the place you actually use the class.

    The fix should be quite straightforward - just move the logic out of the constructor. You could use a Lazy<T> to do this for example:

    public class UserService
    {
        public readonly Lazy<string> _userId ;
    
        public UserService(IHttpContextAccessor context)
        {
            _userId = new Lazy<string>(() => 
            {
                if (!context.HttpContext.Request.Headers.TryGetValue("x-test", out var user))
                {
                    throw new ArgumentException($"x-test header is required.");
                }
                
                return user;
            });
    
            //do other stuff
        }
    
        public string UserId => _userId.Value;
    }
    

    Or you could just get the value when you needed it:

    public class UserService
    {
        public readonly IHttpContextAccessor _context;
    
        public UserService(IHttpContextAccessor context)
        {
            _context = context;
            //do other stuff
        }
    
        public string UserId
        {
            get
            {
                if (_context.HttpContext.Request.Headers.TryGetValue("x-test", out var user))
                {
                    return user;
                }
                else
                {
                    throw new ArgumentException($"x-test header is required.");
                }
            }
        }
    }