Prevent scheduling polling attempts that would exceed timeout

[asshoffm]

Hi @SergeyPetrachkov,

thanks for this async implementation of polling!

While integrating it, I noticed that the timeout logic allows one additional iteration to start even if it would cause the polling to exceed the timeout.

So, this pull request ensures that the timeout is strictly respected by considering the polling interval when deciding whether to schedule the next attempt.

Let me know what you think! 🙂

[SergeyPetrachkov]

Thanks for the contribution!

A few tiny things to check:

• let's not put timeout intervals too big, ideal tests should be very fast :) if this thingy is used by any team/app, having a test that runs for ~2 seconds is going to have a huge impact on the CI and the bill in the long run :) Let's keep it as fast as possible :)

There's another issue with the @Test("Already Polling"). It now fails like in 0.1% cases (if you run it repeatedly for a 1000 times, you'll get the same result). But that part is not on you or me, it's something to do with the Swift Testing. As soon as I do the same with the XCTest, all becomes stable.

So, I'd say: let's adjust the polling config for @Test("Polling timeout") and I'll merge this one :)

After that I'll tackle the other part with the tests.

Thanks again for the contribution!

[asshoffm]

It now fails like in 0.1% cases

That's why I adjusted the intervals. But you are right about the longer execution time. I reverted it and removed the check for the interval count. 👍

[asshoffm]

After that I'll tackle the other part with the tests.

We could try to inject a Clock to make it testable? 🙂

[SergeyPetrachkov]

it requires iOS 16 though, I wanted to support lower versions as well 🤔

[SergeyPetrachkov]

I'm kinda having second thoughts about this one :D So, which logic is actually fair for a poller: start a new iteration before the timeout it reached or make sure that the job is finished before the timeout is reached? :D
if the former, then the initial logic was correct. if the latter, then the your patch is correct.

Tests flakiness has something to do with Task.sleeps. Those are not precise and Xcode acts differently from running tests from command line. Always a pleasure working with unstable dev tools 🙈

[SergeyPetrachkov]

So:

• If the goal is to not exceed the total timeout even with the last job running, the initial version is better.
• If the goal is to not start a new attempt when the timeout is already near, your version is better.

Most real-world polling scenarios I've seen lean towards the "start before timeout" logic because it’s cleaner and easier to reason about. I think this can be made an option in the configuration.

[asshoffm]

If the goal is to not exceed the total timeout even with the last job running, the initial version is better.

But where exactly is that ensured? Don't both implementations ignore the execution time of the (last) job itself and differ only in how it's executed?

• The original implementation waits and starts another poll, even if the polling interval would cause the timeout to be exceeded
• The new version checks whether the next poll can begin within the allowed window and avoids scheduling if it would exceed the limit

[SergeyPetrachkov]

We don't consider the execution time of the job indeed. I chose incorrect words. I meant to say that we allow the last attempt to start even if it's 0.000000001sec before the timeout. Which kinda makes sense for all business cases. If I'm polling an API, a request can take a lot of time (depending on connectivity and server itself), but that's ok for a client I guess. Projects that I worked on specified the requirements as "to keep asking for a response in the next 5 minutes with intervals of 30 seconds" or something like that.
We could also work on injectable polling strategies stricter/more relaxed.