TrueTime.swift icon indicating copy to clipboard operation
TrueTime.swift copied to clipboard

How does TrueTime respond to Reachability changes?

Open warpling opened this issue 9 years ago • 9 comments

It seems that if retrieveReferenceTimeWithSuccess:failure: is called while internet is offline it does not reattempt when it comes online? How is it expected to behave with reachability events?

warpling avatar Nov 11 '16 00:11 warpling

Once start is called, TrueTime internally starts fetching a reference time, and automatically pauses/restarts upon receiving reachability events. The retrieveReferenceTime function blocks waiting for the fetch to complete. If we already have a valid time (most common scenario), it returns a value immediately; otherwise, it will either wait for the outstanding requests to complete or return an error if they can't be made (e.g. network is offline).

In our app, we use this to show a spinner when fetching the reference time is still in progress. We don't want to show an infinite spinner when offline, so instead we show a prompt requiring an internet connection the first time a network time is sync'd.

msanders avatar Nov 14 '16 19:11 msanders

Gotcha. So to clarify, are all of the following true?

  • start should only be called once unless it failed previously.
  • If the device is offline when start is called it will fail immediately and provide an error to the retrieveReferenceTime block.
  • The reference time is not cached in any way; so just because start successfully gets a reference time during an app launch, does not mean it won't need to be online to retrieve it again the next time the app is launched from a cold start and start is called again.

warpling avatar Nov 14 '16 22:11 warpling

  • start only needs to be called once (e.g. at app launch). The library will automatically pause/restart depending on network conditions. There's no need to call it again unless it's manually paused.
  • If the device is offline when waiting to fetch with retrieveReferenceTime and no time is cached, it will error out immediately. (perhaps this is poorly named, maybe fetchIfNeeded would be better?)
  • Times are cached in-memory, but eventually need to be invalidated to compensate for clock drift on devices (right now ~8 minutes). We're planning on adding support for persisting to disk as well (see #9), but still need to fetch at least one response from the server and sync periodically to ensure accurate times.

We can add functionality to support what you're describing if it would be helpful. Would it preferable for your use-case to have a function that keeps waiting until it gets a valid response, even when offline?

msanders avatar Nov 15 '16 00:11 msanders

For naming inspiration: my own implementation used a method named startPollingIfNecessary.

  • Got it 👍
  • It's hard to grok the difference between retrieveReferenceTimeWithSuccess:failure: and start though. I assume most devs that just want a replacement for [NSDate date] will be using [[client referenceTime] now], and those who want the time once and as soon as possible use retrieveReferenceTimeWithSuccess. Perhaps it might be clearer if start had a consolidated startRetrievingTimeWithSuccess:Failure: variant? (Then again maybe my understanding of how the API is intended to be used is off)
  • I was wondering why my own implementation had so much drift! I think it'd be best to have an expiry date for the reference time and when expired (or invalid due to reboot) attempt to get a new reference time, while holding onto and and using the current one until a new value is retrieved (unless say it's far past some expiry threshold).

My use-case currently is to prevent cheating. To do this I use TrueTime to get a time once at start-up and then create my own association (With boot/uptime) that I cache across app restarts until it is deemed expired or invalid. I currently watch for reachability events and lifecycle events to start new requests if previous ones have failed. Ideally I could stop doing my own association stuff and just rely solely on TrueTime to acquire its own reference time as quickly as possible and serve as a replacement to [NSDate date]. :)

warpling avatar Nov 15 '16 20:11 warpling

Hiya. I will have to think about this some more. The reasoning behind it being a separate method from start is to distinguish between starting the NTP sync (which can be done asynchronously at any time, e.g. at the launch of the app) and blocking to wait for a fetch. But I see what you're saying about just wanting a callback that returns when we have a valid time.

I believe what you're describing re: auto-updating reference times is currently supported. Each ReferenceTime will automatically get updated under the hood once we are online and past its expiration date.

For the time being, I've renamed this to fetchIfNeeded to add some clarity, but I'll be thinking about this use-case some more.

msanders avatar Nov 22 '16 02:11 msanders

That seems fine for start then, especially since libraries often require some sort of initialize call before being used.

I think that adds some clarity. So fetchIfNecessary should only need to be called once per launch unless it fails, but is safe to call via lifecycle methods like didBecomeActive?

warpling avatar Nov 22 '16 03:11 warpling

:+1: Yep that's right, or it can be called multiple times, in which case it will just return immediately with the cached time.

msanders avatar Nov 22 '16 03:11 msanders

@msanders so if start is called at didFinishLaunching but theres no network, client.referenceTime?.now() will be nil?

otymartin avatar Feb 18 '17 11:02 otymartin

@otymartin That is correct, until #9 is implemented.

msanders avatar Feb 21 '17 20:02 msanders