1

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.

1 Answer 1

0

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);
    }
}
Sign up to request clarification or add additional context in comments.

4 Comments

Oddly enough, neither one of these solutions appears to correct the issue. I've tried both adding the async keyword and setting the exception manually myself, and in both cases the results are identical. I've even gone as far as manually just throwing an exception directly in the controller, and still the middleware catch block is never hit.
Weird. Does it behave the same if you define your middleware "inline" like app.Use(async (context, next) =>?
It does. I started trying to create an example project, but was unable to replicate it in a new project. So I guess there's something else somewhere I'm not noticing.
Ah, it turns out we had a custom attribute on our BaseController that was hijacking all exceptions for logging purposes.

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.