Open martincostello opened 1 month ago
Your proposed solution (as far as I understand) works with single exception type only. You can't chain them.
IMHO having different classes for simple and advanced scenarios does not help the usage of the API.
If we want to rewrite the predicate builder then we should look at how others have address this problem.
Like resilience4j or guava-retrying.
Your proposed solution (as far as I understand) works with single exception type only. You can't chain them.
Exactly - this suggestion is about making the simplest cases easier where you only want to check for one thing, not many.
It's just on reflection seems a bit overly-verbose and wasteful to allocate an object and then immediately throw it away. I guess if you cared enough about that you could just write a delegate directly instead, but then you need to account for the ValueTask and Outcome machinery so then it ends up being more complicated.
We have all the pieces, they're just hidden away in the internals.
Hey @martincostello
I think making this work will be more difficult, even not possible.
The thing is that the predicates are more complex than Predicate<Outcome<TResult>>
and assigning it to RetryOptions.ShouldHandle
is not possible. For example, this is the signature of RetryOptions.ShouldHandle
predicate:
Func<RetryPredicateArguments<object>, ValueTask<bool>>
Assigning PredicateBuilder
to this value works because we define a custom converter to this delegate:
https://github.com/App-vNext/Polly/blob/badf0df4dd735fa7f2281c3a95f1d46cf42bd623/src/Polly.Core/PredicateBuilder.Operators.cs#L18
Now, we cannot do the same for Predicate<Outcome<TResult>>
, I have actually tried before :)
It's just on reflection seems a bit overly-verbose and wasteful to allocate an object and then immediately throw it away. FYI, this is "do once and never again" as it happens only when initializing the pipeline. There are no allocations on hot path.
Is your feature request related to a specific problem? Or an existing feature?
Doing some changes to an application of my own, it occurred to me that for basic use cases for retries, the syntax is more verbose than it could be:
The
PredicateBuilder
class is a good way to express complex conditions, but needing to allocate the object (and the underlying array) to then create the delegate to assign toShouldHandle
seems a bit over-the-top.It feels like we could add something that's a bit simpler (and then implement
PredicateBuilder
in terms of it).This is just a rough sketch of an idea to see what people think for now, I haven't tried to actually prototype it yet or anything.
Describe the solution you'd like
Something like this, where the existing instance methods in the
PredicateBuilder
would just call new static methods, but simple cases can use them directly:That would then simplify the original example to something like this:
Additional context
No response