Move CTS closer to its usage #10191
Move CTS closer to its usage #10191
Conversation
| using CancellationTokenSource requestTimeoutCts = new(ApiRequestTimeout); | ||
| using CancellationTokenSource linkedCts = CancellationTokenSource.CreateLinkedTokenSource(verificationCancellationToken, item.Token, requestTimeoutCts.Token); | ||
|
|
There was a problem hiding this comment.
This code looks correct but I would suggest to use this: https://github.com/zkSNACKs/WalletWasabi/blob/master/WalletWasabi/Extensions/CancellationTokenExtensions.cs
Basically, instead of having the same kind of block repeated again and again, you can replace it with this:
c# using var linkedCts = verificationCancellationToken.CreateLinkedTokenSourceWithTimeout(ApiRequestTimeout);
There was a problem hiding this comment.
I didn't know this extension method existed. Ty, I will keep this in mind for the future.
But here I need to link 3 different cancellation token together, the extension method doesn't help to make the code prettier in this method IMO.
|
It changed the behavior but I think this one is better than the one before. |
Mistake I made in this PR #10174
requestTimeoutCts(3 minutes) started ticking when it still had a 2 minute delay, resulting in an early cancellation.Even tho the API provider usually respond in less ~10 seconds, it's better to have these CTS in its proper place.