Add overloads taking CancellationToken, for async configuration-providing delegates

[reisenberger]

As @martincostello mentions here, given the Simmy API does offer async overloads to obtain configuration values in the async cases, those APIs might want take to a CancellationToken.

We would need to agree what the assumptions should be if cancellation was signalled for a config-obtaining delegate:

• for delegates returning bool enabled, we would probably have no option but to assume cancellation means we return false
• for delegates returning injectionRate or Timespan we would probably likewise have no better option than to assume cancellation means the fault-injection operation should default to not injecting the fault

Thoughst?

---

x-ref: #18 affects same APIs

[vany0114]

Yes, absolutely agree, We can't assume a default value for injectionRate or enabled parameters (other than 0 and false). @reisenberger when you say Timespan you mean the delegate to get the latency value and not the latency injected itself that we're going to implement in #20 right? if so, we need to do so with Exception/TResult as well

[reisenberger]

when you say Timespan you mean the delegate to get the latency value [...] if so, we need to do so with Exception/TResult as well

Hi @vany0114 . Yes, exactly that.

Actually, it looks like there are already overloads configuring delegates which take CancellationToken for obtaining the latency, the fault exception, fault result and behaviour configurations, so it is really just the Func<... Task<T>> format for injectionRate and enabled (like this but throughout) which maybe could benefit from the addition of CancellationToken overloads.

What do you think?

I have learnt with Polly that - with the current syntax model - it is easy for numbers of overloads to proliferate by adding new possibilities. Worth considering not to add every possible combination, or even whether to replace existing most-complex overloads with ones which also take a CancellationToken input parameter in those cases.

(The longer-term solution to the many-configuration-overloads problem is to change the syntax to .PolicyType(Action<IPolicyTypeConfiguration>), which allows new features to be added more flexibly. I plan this for Polly sometime. You could experiment with this for Simmy now if you prefer!)

[reisenberger]

Another question arises: if the delegates to asynchronously retrieve configuration for latency, Exception or TResult can take CancellationToken - what CancellationToken value should the Simmy policies actually pass when executing those configuration delegates?

(i) The CancellationToken passed through the Polly execution as a whole (passed in to the .ExecuteAsync(...) where Polly is used) governs cancelling the execution as a whole. It would make sense for configuration delegates at least to honour this.

(ii) But that's not the same as a short timeout on obtaining chaos configuration. If we want to add the feature of a short timeout on obtaining chaos configuration, then either:

• (a) we also have to extend all the Simmy APIs to receive that timeout, and combine in a timing-out cancellation token with the token from (i) above; OR
• (b) we say that timeout is something that Polly already handles well, with TimeoutPolicy, and users can just use existing Polly TimeoutPolicy within their configuration delegate.

Thoughts?

[vany0114]

(The longer-term solution to the many-configuration-overloads problem is to change the syntax to .PolicyType(Action), which allows new features to be added more flexibly. I plan this for Polly sometime. You could experiment with this for Simmy now if you prefer!)

Hi @reisenberger, I agree, I remember you told me about that idea before, and I think it's great! since it will allow Simmy to grow easier, so having said that, once we finished with that new syntax, I think we can implement the scenario you explained above (receiving a timeout for configuration delegates --should be easier then--) and for now, just let people handle it using Polly.

In the meantime, I can create a PR to allow the injectionRate and enabled guys are cancellable. What do you think?

P.S: after that, I can start to work on the new syntax.

[reisenberger]

@vany0114 , Sounds like a great plan, please go ahead! It's awesome working with you on Simmy and we are really happy for you to drive ideas and make recommendations like this. 💯 👍 🙏

[vany0114]

My pleasure @reisenberger, I'm very happy working with you guys, I'm learning a lot!