1

I have a piece of code that is intended to act as middleware in charge logging timestamps before and after the execution of an arbitrary piece of code, as well as catching any exception it may throw, logging them and rethrowing them. I'm stuck trying to engineer a test that makes sure the middleware does not affect the stacktrace the original code threw.

I've tried this so far

[Fact]
public async Task The_Middleware_Must_Rethrow_Any_Exception_Thrown_By_The_Next_Middleware()
{
    var middleware = new ErrorMiddleware();
    _testSubject = new RequestLoggerMiddleware(middleware);

    try
    {
        await _testSubject.Invoke(_mockContext.Object);
    } catch (Exception ex)
    {
        Assert.Same(ex, middleware.ThrownException);
        Assert.Equal(ex.StackTrace, middleware.ThrownException.StackTrace);
        return;
    }

    Assert.True(false, "The middleware did not rethrow the exception");
}

private class ErrorMiddleware : OwinMiddleware
{
    public Exception ThrownException { get; }

    public ErrorMiddleware() : base(null) 
    {
        ThrownException = new Exception(TestExceptionMessage);    
    }

    public override Task Invoke(IOwinContext context)
    {
        throw ThrownException;
    }
}

It seems to work initially, but if I make RequestLoggerMiddleware run throw ex; on its catch (Exception ex) block (rewriting the stacktrace in the process) the test will still pass, even though it shouldn't at all.

Machinarius
  • 3,637
  • 3
  • 30
  • 53
  • 2
    this seems to be something a static analyzer should catch instead. – Daniel A. White May 17 '19 at 15:00
  • There was an extension for this. It was buggy and crashed a lot, but it was a great way to find issues like that. Here's something else that analyzes for code that mishandles exceptions. I prefer the analyzer approach because a) who wants to write tests for that and b) If we need to check for this it's probably all over our code. I'm going to try this out right now. https://www.nuget.org/packages/CleanCodeNet – Scott Hannen May 17 '19 at 15:16
  • Does just doing `throw;` (rather than `throw ThrownException;`) not working for you? See this: https://stackoverflow.com/a/730255/809357 – trailmax May 17 '19 at 15:17
  • @trailmax You can do `throw;` in a catch block. But this isn't in a catch block. – mason May 17 '19 at 15:22
  • 2
    Another way of looking at this is to question whether we need to test it at all. The behaviors of `throw` vs `throw ex` are known. Is a test needed to confirm their behavior? We write unit tests for what we can control. If the test failed, we wouldn't be able to do anything to correct it. – Scott Hannen May 17 '19 at 15:25
  • @mason good point. I've missed that. – trailmax May 17 '19 at 22:09

1 Answers1

0

It might be that exception is being wrapped somewhere inside _testSubject. If this wrapping is unacceptable, a specific method which throws exception could be checked like:

    Assert.Equal(
            ex.TargetSite, 
            typeof(ErrorMiddleware).GetMethod(nameof(ErrorMiddleware.Invoke), BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic));
Renat
  • 7,718
  • 2
  • 20
  • 34
  • @Machinarius , ok, then I'm making another guess. What if middleware wraps the exception. i've updated my answer – Renat May 17 '19 at 15:57