Skip to content
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

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

phausler
Copy link
Member

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.

@phausler phausler requested a review from FranzBusch September 15, 2023 17:27
@phausler
Copy link
Member Author

@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)
Copy link
Member

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?

Copy link
Member Author

@phausler phausler Sep 18, 2023

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.

Copy link
Member

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.

Copy link
Member Author

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

Copy link
Member

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

@FranzBusch FranzBusch added the v1.0 Work leading up first API stable version label Sep 21, 2023
@phausler phausler merged commit 220f86f into apple:main Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.0 Work leading up first API stable version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants