feat!: add CancellationTokens, ValueTasks hooks

[toddbaert]

This PR is a combination of #184 and #185. Changes include:

• adding cancellation tokens
  • in all cases where async operations include side-effects (setProviderAsync, InitializeAsync, I've specified in the in-line doc that the cancellation token's purpose is to cancel such side-effects - so setting a provider and canceling that operation still results in that provider's being set, but async side-effect should be cancelled. I'm interested in feedback here, I think we need to consider the semantics around this... I suppose the alternative would be to always ensure any state changes only occur after async side-effects, if they weren't cancelled beforehand.
• adding "Async" suffix to all async methods
• remove deprecated sync SetProvider methods
• Using ValueTask for hook methods
  • I've decided against converting all Tasks to ValueTasks, from the official .NET docs:

  the default choice for any asynchronous method that does not return a result should be to return a Task. Only if performance analysis proves it worthwhile should a ValueTask be used instead of a Task.

  • I think for hooks, ValueTask especially makes sense since often hooks are synchronous, in fact async hooks are probably the less likely variant.
  • I've kept the resolver methods as Task, but there could be an argument for making them ValueTask, since some providers resolve asynchronously.
  • I'm still a bit dubious on the entire idea of ValueTask, so I'm really interested in feedback here
• associated test updates

UPDATE:

After chewing on this for a night, I'm starting to feel:

• We should simply remove cancellation tokens from Init/Shutdown. We can always add them later, which would be non-breaking. I think the value is low and the complexity is potentially high.
• ValueTask is only a good idea for hooks, because:
  • Hooks will very often be synchronous under the hood
  • We (SDK authors) await the hooks, not consumer code, so we can be careful of the potential pitfalls of ValueTask. I think everywhere else we should stick to Task.

[askpt]

I left a few comments. It is a big diff, but most of it is just renaming.

Overall it looks good but I added some of my concerns.

[toddbaert]

After chewing on this for a night, I'm starting to feel:

• We should simply remove cancellation tokens from Init/Shutdown. We can always add them later, which would be non-breaking. I think the value is low and the complexity is potentially high.
• ValueTask is only a good idea for hooks, because:
  • Hooks will very often be synchronous under the hood
  • We (SDK authors) await the hooks, not consumer code, so we can be careful of the potential pitfalls of ValueTask. I think everywhere else we should stick to Task.

[kinyoklion]

I think that we should probably have tests which have a dummy async provider that blocks evaluations until cancelled, then we should start an evaluation and cancel it to make sure all the steps happen. Maybe something similar with a hook implementation.

[toddbaert]

I think that we should probably have tests which have a dummy async provider that blocks evaluations until cancelled, then we should start an evaluation and cancel it to make sure all the steps happen. Maybe something similar with a hook implementation.

That's a good idea. Will add.

[toddbaert]

I think that we should probably have tests which have a dummy async provider that blocks evaluations until cancelled, then we should start an evaluation and cancel it to make sure all the steps happen. Maybe something similar with a hook implementation.

That's a good idea. Will add.

I've added tests for cancellation (evaluation/resolution and hooks): 036d7a4

[askpt]

I left a few suggestions around the tests.
We should await the async operations, and we should not use async void but async Task.

[askpt]

Resetting my approval. Sorry! 😃