[Bug]: Undocumented breaking changes from Policy.HandleInner to PredicateBuilder.HandleInner

[cmeeren]

Describe the bug

I just migrated from the old Policy API to the new ResiliencePipeline API. I read the migration guide. It does not mention HandleInner. However, v8 HandleInner handles exceptions only at the first inner level (only the top level exception's InnerException), whereas v7 HandleInner handled exceptions at any level. This bit us in prod since we assumed that the behavior was the same (again, no change was documented). So I am assuming that this is a bug.

Expected behavior

The new API should work the same as the old one.

Actual behavior

See below.

Steps to reproduce

This simple F# script demonstrates the issue:

open System
open Polly

let testCases : (string * exn) list =
    [
        "outer   ", InvalidOperationException()
        "inner1  ", Exception("", InvalidOperationException())
        "inner2  ", Exception("", Exception("", InvalidOperationException()))
        "agg     ", AggregateException("", InvalidOperationException())
        "agg2    ", AggregateException("", Exception(), InvalidOperationException())
        "aggInner", AggregateException("", Exception("", InvalidOperationException()))
    ]

for name, ex in testCases do
    let mutable innerHandled7 = false
    let inner7 = Policy.HandleInner<InvalidOperationException>().Fallback(fun () -> innerHandled7 <- true)
    try inner7.Execute(Action(fun () -> raise ex)) with _ -> ()
    
    let inner8 = PredicateBuilder().HandleInner<InvalidOperationException>().Build()
    let innerHandled8 = inner8.Invoke(Outcome.FromException(ex))

    printfn $"case={name}  handled_v7=%-6b{innerHandled7}  handled_v8=%b{innerHandled8}"

Output:

case=inner1    handled_v7=true    handled_v8=true
case=inner2    handled_v7=true    handled_v8=false
case=agg       handled_v7=true    handled_v8=true
case=agg2      handled_v7=true    handled_v8=false
case=aggInner  handled_v7=true    handled_v8=false

Exception(s) (if any)

No response

Polly version

8.4.0

.NET Version

8.0.302

Anything else?

No response

[martincostello]

@martintmk Am I right that this was intentional and we just forgot to document it?

[martintmk]

@martintmk Am I right that this was intentional and we just forgot to document it?

I think this was just omission, I haven't realized all inner exceptions should be handled. I think we can backport this behavior, wdyt?

[martincostello]

If it's a mistake, then yeah we should fix it.

[cmeeren]

I hope so, because I don't think the current behavior (only the first .InnerException and nothing else) is useful (and is unexpected given the Policy behavior).

[github-actions]

Thanks for creating this issue @cmeeren - the associated changes have been published as part of version 8.4.1 📦, which is now available from NuGet.org 🚀