Feature Request: Add support for recursive checking of inner exception

Description

Sometimes, when writing high-level tests, we expect a function to throw an exception with a deeply nested inner exception. Writing assertions can be tedious if we only care about the existence of a certain type in the chain and not how deep it is located.

So in short, I would like to write something like this:

action.Should().Throw<ExceptionA>()
    .WithInnerException<ExceptionB>(recursive: true);

instead of:

action.Should().Throw<ExceptionA>()
    .Which.InnerException.InnerException.InnerException.BeOfType<ExceptionB>();

Actually, I have written an extension method for this functionality. If you are interested and this is correct, I can submit a pull request.

public static ExceptionAssertions<TInnerException> WithInnerException<TException, TInnerException>(
    this ExceptionAssertions<TException> exceptionAssertions,
    bool recursive = false,
    string because = "",
    params object[] becauseArgs)
    where TException : Exception
    where TInnerException : Exception
{
    Execute.Assertion
        .BecauseOf(because, becauseArgs)
        .WithExpectation("Expected inner {0}{reason}, but ", typeof(TInnerException))
        .ForCondition(exceptionAssertions.Subject is not null)
        .FailWith("no exception was thrown.")
        .Then
        .ForCondition(exceptionAssertions.Subject!.Any(e => e.InnerException is not null))
        .FailWith("the thrown exception has no inner exception.")
        .Then
        .ClearExpectation();

    TInnerException[] expectedInnerExceptions =
        (recursive
            ? exceptionAssertions.Subject!
                .Select(e => GetRecursiveInnerExceptions(e).OfType<TInnerException>().FirstOrDefault())
            : exceptionAssertions.Subject!
                .Select(e => e.InnerException))
        .OfType<TInnerException>()
        .ToArray();

    Execute.Assertion
        .ForCondition(expectedInnerExceptions.Any())
        .BecauseOf(because, becauseArgs)
        .FailWith(
            "Expected inner {0}{reason}, but found {1}.",
            typeof(TInnerException),
            exceptionAssertions.Subject!.Single().InnerException);

    return new ExceptionAssertions<TInnerException>(expectedInnerExceptions);

    static IEnumerable<Exception> GetRecursiveInnerExceptions(
        Exception ex)
    {
        var current = ex.InnerException;
        while (current is not null)
        {
            yield return current;
            current = current.InnerException;
        }
    }
}

I'm trying to come up with a good name for that overload. Something like WithInnerExceptionAnywhere. Ideally we would want to make that the default, but then you would need a WithDirectInnerException. And that would be a breaking change as well.

Interesting use case, I definitely see why you want a higher-level assertion. I'm not sure the recursive search should be the default behavior of WithInnerException, as it might be slightly misleading for those who want to assertion on the immediate inner exception.

Maybe look at XElement type of naming? Something like WithDescendantInnerException

Would action.Should().Throw<ExceptionA>().ContainingInnerException<ExceptionB>() make sense?

More ideas through this Twitter feed:

• WithNestedInnerException
• WithDeepInnerException

Maybe the code should also consider AggregateExceptions which have an additional collection member InnerExceptions.

Another possibility for this would be to convert it into a normal collection assertion with something like:

action.Should().Throw<ExceptionA>().WhoseInnerExceptionChain.Should().Contain<ExceptionB>();

This would be similar to the WhoseValue used in places like dictionaries and leverage all the collection assertions that already exist on top of the flattened list of exceptions (that could also be empty). It could also be a computed IEnumerable (i.e. deferred execution), depending on how we want to go about it.

I think the "Chain" wording at the end makes sense, but perhaps there is a better way to put it (perhaps just WhoseInnerExceptions, even though that could be a bit misleading when dealing with an AggregateException).

From what I'm seeing in the docs, it seems we don't have a generic Contain<T> assertion, so I'd probably add that as well to make this work, since it would be a valid assertion to have for normal collections too (would be useful for polymorphic collections). Naming could either be Contain<T>, or maybe a more explicit ContainItemOfType<T> to go along the other "Contain...Items..." methods. While we are at it, a "Single" version should also be added to match.

I think that's a great idea and I would prefer WhoseInnerExceptionChain to emphasize that that it exposes not just the direct inner exceptions.

Hi

Is this ticket kind of "api-approved" and can be implemented? Or do we need something else here?

My proposed API changes are:

GenericCollectionAssertions<T>
{
+         public FluentAssertions.AndConstraint<TAssertions> ContainItemsOfType<TExpectation>(string because = "", params object[] becauseArgs) { }
+         public FluentAssertions.AndWhichConstraint<TAssertions, T> ContainSingleOfType<TExpectation>(string because = "", params object[] becauseArgs) { }
}
ExceptionAssertions<TException>
{
+         public System.Collections.Generic.IEnumerable<TException> WhoseInnerExceptionChain { get; }
}

But I'm a bit unsure, wether name it ContainItemsOfType or ContainItemsOfExactType..

I'm all for WhoseInnerExceptionChain, but I don't think we should add those two ContainXX methods considering how many of those assertion methods we already have, including ContainItemsAssignableTo.

hmm..

What about giving ContainItemsAssignableTo an overload which checks the type exactly?

As I understand the ContainItemsAssignableTo (which calls IsAssignableFrom) correctly: it matches also derivatives of a type T, but I don't think this is what we want here, do we?

So typeof(object).IsAssignableFrom(typeof(TException)) would be true...

Or (more real world scenario):

var exceptions = new List<Exception>();
exceptions.Add(new NullReferenceException());
exceptions.Add(new FormatException());

exceptions.Should().ContainItemsAssignableTo<Exception>(); // would pass, but I would expect it to fail

Yeah, that's true. But then you need ContainItemsOfExactType and ContainSingleOfExactType.

That's why I proposed this two new Contain...<T> methods.

What about something like:

action.Should().Throw<ExceptionA>().WhoseInnerExceptionChain.Should().HaveElementsOfExactType<ExceptionB>();

// or

action.Should().Throw<ExceptionA>().WhoseInnerExceptionChain.Should().HaveElementsOfType<ExceptionB>();

// or

action.Should().Throw<ExceptionA>().WhoseInnerExceptionChain.Should().HaveItemsOfType<ExceptionB>();

Using GenericCollectionAssertions here makes the most sense IMHO.

Another approach could be:

public class ExceptionChain
{
        internal IEnumerable<Exception> elements;

        public Has<T>();
}

public class ExceptionAssertion<TException>
{
        public ExceptionChain WhoseInnerExceptionChain { get; }
}

action.Should().Throw<ExceptionA>().WhoseInnerExceptionChain.Has<ExceptionB>();

I prefer HaveElementsOfType<ExceptionB>

Ok

And for the single overload? (If we address this here too)

action.Should().Throw<ExceptionA>().WhoseInnerExceptionChain.Should().HaveElementOfType<ExceptionB>();

Or using the neat little thing called OccurrenceConstraint 😎

so this would then be for example:

action.Should().Throw<ExceptionA>().WhoseInnerExceptionChain.Should().HaveElementsOfType<ExceptionB>(Exactly.Once()); // for both

But in this case I would use HaveElementOfType<Foo>(Exactly.Once());... reads more fluent IMHO

Or using the neat little thing called OccurrenceConstraint 😎

so this would then be for example:

action.Should().Throw<ExceptionA>().WhoseInnerExceptionChain.Should().HaveElementsOfType<ExceptionB>(Exactly.Once()); // for both

But in this case I would use HaveElementOfType<Foo>(Exactly.Once());... reads more fluent IMHO

Is this actually idiomatic FluentAssertions? I don't remember seeing it in other overloads but I might just be misremembering.

EDIT: Ok I see it is very rarely used? How odd... it actually sounds pretty well thought out! I'd expect this to be quite pervasive.

Was introduced in #1145 3 years ago

Was introduced in #1145 3 years ago

That's..... very surprising. Makes me wonder why it is used so sparingly. I imagine some methods could be deprecated in favor of that even.

Or using the neat little thing called OccurrenceConstraint

Hmm, that might not be a bad idea indeed. Wondering what @jnyrup thinks about that.

If we could change ContainItemsAssignableTo<TExpectation> to return an AndWhichConstraint<TAssertions, IEnumerable<TExpectation>> we could do this.

action.Should().Throw<Exception>()
    .WhoseInnerExceptionChain.Should().ContainItemsAssignableTo<InvalidOperationException>()
        .Which.Should().HaveCount(42);

But I'm not sure this is possible since we also support checking whether a Type is assignable to another Type. And it doesn't read that well.

Another possibility would be to add an overload of ContainItemsAssignableTo that took an OccurrenceConstraint.

action.Should().Throw<Exception>()
    .WhoseInnerExceptionChain.Should().ContainItemsAssignableTo<InvalidOperationException>(Exactly.Times(42));

Some more brainstorming on a name for WhoseInnerExceptionChain as I'm not (yet) completely sold on the "Chain" part of the name.

• WhoseNestedExceptions
• WhoseNestedInnerExceptions
• WhoseFlattenedExceptions (inspired by AggregateException.Flatten())

I like WhoseInnerExceptionChain and WhoseNestedException equally, but I don't like the use of ContainItemsAssignableTo. Not just because it doesn't read very nicely, but also because we most likely care about the exact type.

Hmm.. that limits the possibilities of a fluent syntax extremely..

But what if we omit the Should() after WhoseInnerExceptionChain?

So for example:

action.Should().Throw<Exception>()
    .WhoseInnerExceptionChain.ContainsWithExactType<InvalidOperationException>(Exactly.Times(42));

or something like this..

Can you @dennisdoomen @jnyrup please clarify wether the way of finally asserting a generic collection is the right one.

or should we concentrate on asserting only a IEnumerable<Exception> here?

@ITaluone I'm not sure what you're asking for? I'm still in a brain storming phase exploring different shapes for the feature, both in terms of functionality and naming.

What I meant was: Should we concentrate only on exceptions (and inner exceptions) or should this feature also affect all collections.

If WhoseInnerExceptionChain returns an IEnumerable<Exception> and we want to be able to write action.Should().Throw<SomeException>().WhoseInnerExceptionChain.Should().XYZ() I guess we have to look at both exceptions and collections in general at the same time.

Maybe except if we make XYZ() a dedicated extension method on top of GenericCollectionAssertions<T> where T : Exception.

I think a dedicated extension method would fit better into the structure of the GenericCollectionAsstertions to not pollute this class, because now I would have to take care if the type is an exception.

So I have to do this:

if(T is Exception) 
{
        Assert.Exceptions...
}
else // any other type
{
        Assert.AnyOtherType
}

And writing where T : Exception inside the GenericColltionAssertions is not quite generic :D