Exceptions not bubbling up to Error Handling Middleware?

Question

I have a piece of middleware that is supposed to catch my exceptions and properly set the Http Response code as exceptions occur, but it appears that no matter what I do, I'm still getting an OK response.

Here's the middleware

public class ErrorHandlingMiddleware
{
    private readonly RequestDelegate _next;

    /// <inheritdoc />
    public ErrorHandlingMiddleware(RequestDelegate next)
    {
        _next = next;
    }

    /// <summary>
    /// Called by execution pipeline
    /// </summary>
    /// <param name="context"></param>
    /// <returns></returns>
    public async Task Invoke(HttpContext context /* other dependencies */)
    {
        try
        {
            await _next(context);
        }
        catch (Exception ex)
        {
            await HandleExceptionAsync(context, ex);
        }
    }

    private static Task HandleExceptionAsync(HttpContext context, Exception ex)
    {
        var code = HttpStatusCode.InternalServerError; // 500 if unexpected

        var result = JsonConvert.SerializeObject(new { error = ex.Message });
        context.Response.ContentType = "application/json";
        context.Response.StatusCode = (int)code;
        return context.Response.WriteAsync(result);
    }
}

It's added to my startup like so:

public void Configure(IApplicationBuilder app, IHostingEnvironment env)
{
    app.UseMiddleware(typeof(ErrorHandlingMiddleware));
    if (env.IsDevelopment())
    {
        //app.UseDeveloperExceptionPage();
    }else
    {
        app.UseHsts();
    }

    app.UseHttpsRedirection();
    app.UseStaticFiles(new StaticFileOptions
    {
        ServeUnknownFileTypes = true
    });
    app.UseDefaultFiles();
    app.UseCookiePolicy();
    app.UseMvc();

    app.UseCors("CorsPolicy");

    app.UseMvcWithDefaultRoute();

    app.UseSwaggerAndUI(Configuration)
       .UseCustomHealthCheck();
}

The code throwing the error is:

public Task<string> SaveFileAsync(string path, byte[] file, string fileType, CancellationToken cancellationToken = default)
{
    var filename = _filenameProvider.GetFilename(path, fileType);
    var fullPath = _fileSystem.Path.Combine(path, filename).Replace('/', '\\');

    try
    {
        _fileSystem.Directory.CreateDirectory(fullPath);
        // Error in the FileSystem abstraction library: https://github.com/System-IO-Abstractions/System.IO.Abstractions/issues/491
        //await _fileSystem.File.WriteAllBytesAsync(fullPath, file, cancellationToken);

        _fileSystem.File.WriteAllBytes(fullPath, file);

        return Task.FromResult(filename);
    }
    catch (Exception ex)
    {
        Log.Error(ex.Message, nameof(SaveFileAsync), _userId);

        throw;
    }
}

And the controller is:

public class PatientDocumentController : BaseController
{
    private readonly IPatientFilePusher _patientFilePusher;


    /// <inheritdoc />
    public PatientDocumentController(IPatientFilePusher filePusher)
    {
        _patientFilePusher = filePusher;
    }

    /// <summary>
    /// Pushes a patient file to the emr
    /// </summary>
    /// <param name="request">Contains the file data.</param>
    /// <param name="token">A auto-generated token that allows for halting execution.</param>
    /// <returns>Ok when complete.</returns>
    [HttpPost]
    public async Task<IActionResult> PushPatientDemographicsAsync([FromBody] FilePushRequest request, CancellationToken token)
    {
        await _patientFilePusher.PushFileAsync(request, token);

        return Ok();
    }
}

The response body that comes back includes the exception, but the Http Status code stays 200. The catch branch on my middleware is never called.

Answer 1

You have a function that has an asynchronous signature but doesn't follow the asynchronous way of doing things:

public Task<string> SaveFileAsync(string path, byte[] file, string fileType, CancellationToken cancellationToken = default)

When a function returns a Task / Task<T>, any exceptions it raises should be captured and placed on that returned task. The async keyword will do this for you.

So, you should either change the function to be async:

public async Task<string> SaveFileAsync(string path, byte[] file, string fileType, CancellationToken cancellationToken = default)
{
    var filename = _filenameProvider.GetFilename(path, fileType);
    var fullPath = _fileSystem.Path.Combine(path, filename).Replace('/', '\\');

    try
    {
        _fileSystem.Directory.CreateDirectory(fullPath);
        // Error in the FileSystem abstraction library: https://github.com/System-IO-Abstractions/System.IO.Abstractions/issues/491
        //await _fileSystem.File.WriteAllBytesAsync(fullPath, file, cancellationToken);

        _fileSystem.File.WriteAllBytes(fullPath, file);

        return filename;
    }
    catch (Exception ex)
    {
        Log.Error(ex.Message, nameof(SaveFileAsync), _userId);

        throw;
    }
}

or place the exception on the returned task yourself:

public Task<string> SaveFileAsync(string path, byte[] file, string fileType, CancellationToken cancellationToken = default)
{
    try
    {
        var filename = _filenameProvider.GetFilename(path, fileType);
        var fullPath = _fileSystem.Path.Combine(path, filename).Replace('/', '\\');

        _fileSystem.Directory.CreateDirectory(fullPath);
        // Error in the FileSystem abstraction library: https://github.com/System-IO-Abstractions/System.IO.Abstractions/issues/491
        //await _fileSystem.File.WriteAllBytesAsync(fullPath, file, cancellationToken);

        _fileSystem.File.WriteAllBytes(fullPath, file);

        return Task.FromResult(filename);
    }
    catch (Exception ex)
    {
        Log.Error(ex.Message, nameof(SaveFileAsync), _userId);

        return Task.FromException<string>(ex);
    }
}