Automattic / Automattic-Tracks-iOS

Client library for tracking user events for later analysis
GNU General Public License v2.0
41 stars 12 forks source link

Ensure network monitor path update handler is called on start #199

Closed mokagio closed 2 years ago

mokagio commented 2 years ago

Earlier today, @yaelirub reported an unusual spike in events logged with the device offline in WordPress iOS starting from 19.2.

Together with @twstokes, the tracked it to the Tracks update to version 0.11.0 and the new network reachability logic from #186. Tanner noticed that, at launch, the network path was nil, resulting in this branch of the network properties logging conditional running.

I experienced the same behavior when running the TracksDemo app from this repo (microapps FTW!).

The behavior I experienced was that the block meant to receive network path updates was not called when the monitor started, even though that's the expectation set by the documentation.

I did some googling but nobody reported the issue. I stumbled onto this post by @madsolar8582 that also explicitly sets a queue on which to dispatch the update block execution. I tried it out and it did the trick 🎉

To test

On trunk, put a breakpoint in the update block: https://github.com/Automattic/Automattic-Tracks-iOS/blob/7fc4afe72cd1539575135891d90bef26ee32106c/Sources/Event%20Logging/TracksService.m#L323

Notice that it's not hit on start, but it is if you change you network interface, for example by turning the Wi-Fi on.

Repeat on this branch and notice that the block is called when the demo app starts (fix) and also when the network changes (no regressions).

Alternatively, use updateDeviceInformationFromReachability as the inspection point for the breakpoint. On trunk here: updateDeviceInformationFromReachability. If you do that, be mindful that it gets called as part of TracksService init, and will have self.networkPath nil there because the monitoring hasn't been setup yet.

bjhomer commented 2 years ago

Thanks for catching this. Sorry I missed it earlier!