Only run the provided delayed operation early #3101
Conversation
Binary Size ReportAffected SDKs
Test Logs |
| * @returns a Promise that resolves once all operations have been run. | ||
| */ | ||
| runDelayedOperationsEarly(lastTimerId: TimerId): Promise<void> { | ||
| runDelayedOperationsEarly(timerId: TimerId): Promise<void> { |
There was a problem hiding this comment.
This breaks the intent of what this test-only method was for. The idea was that you could fast-forward time until the delayed operation ran, including anything scheduled prior to it. In this interface, now you have to know exactly what you want to run, which makes this a much less effective tool for testing. In particular, this makes it really hard to use this for spec tests because we're no longer simulating the actual passage of time, but merely jumping ahead to this specific event. This makes it far less likely that we'll catch scheduling-related problems with tests.
If we really need to allow a scheduled operation to run early, can't we add this as a method to the DelayedOperation?
There was a problem hiding this comment.
I updated the code to return DelayedOperation instead of CancellablePromise, and added a skipDelay to ExponentialBackoff.
I feel that we might even get rid of the TimerIds with this change, since we now have direct access to the underlying operations - but that of course would remove the ability to run delayed operations in order. It might drastically simplify this code though.
schmidt-sebastian
force-pushed
the
mrschmidt/rundelayed
branch
from
0717eeb to
1628f76
Compare
The code to fast-forward the IndexedDB retries uses
runDelayedOperationsEarly(), which runs not only the provided delayed operation, but all operations that were scheduled prior. This causes the OnlineStateTimer to run on page visibility events (see #2923).Since Timer operations are likely not dependent on one another, it seems better to only run the provided operation. This also makes the API much less surprising.
Note that
op.skipDelay()cannot be used directly in the visibility handler since we do not keep track of the DelayedOperation once it is scheduled via the backoff algorithm.Fixes #2923