-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure the last element of reduction in the throttle is emitted and use appropriate delay #292
Conversation
…se appropriate delay
@swift-ci please test |
// ensure the rate of elements never exceeds the given interval | ||
let amount = interval - last.duration(to: clock.now) | ||
if amount > .zero { | ||
try? await clock.sleep(for: amount) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we swallowing the cancellation error here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it cannot throw since that would alter the signature from rethrowing to flat-out throwing.
I guess it could return nil immediately in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if we should throw in the case where we are a throwing sequence and if not just return nil
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't really know that - plus throwing CancellationError
seems rather off to me except in the cases where EOF and cancellation need disambiguation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is impossible to differentiate when a non-throwing async sequence returns nil
if it was due to EOF or task cancellation, that's why I am leaning towards always throwing the CancellationError
if possible. Now this case is interesting because we would just return the reduced value and probably the next iteration will potentially throw the CancellationError
. So I guess it's fine
This corrects and validates the case where the last element of a throttle is within the period of the throttle interval but no subsequent elements are produced after that terminal event.