SwiftSDK
SwiftSDK copied to clipboard
TelemetryDeck
Wait For Connectivity Before Attempting Send
I was just looking over the code and noticed it doesn't check whether the network is up before sending events.
It might be a good idea to configure the session with this property so, instead of blindly sending the request and it failing and then sticking it back in the queue to try again next time, it automatically sends them when the network becomes available.
https://developer.apple.com/documentation/foundation/nsurlsessionconfiguration/2908812-waitsforconnectivity
Very nice point, thanks a lot for pointing that out! I'll see that I'll include that with the next update (if you feel like writing a PR, that's also very welcome)
@winsmith @Megatron1000 I've added it here, I can always submit a PR if needed. It wasn't much code added to support waitsForConnectivity for URLSession.
@winsmith I had a small typo, forgot to remove shared from session, made a quick PR which solves the issue
I think this change drops signals that occur when a device is offline. Signals are popped and attempted to be sent, and if there is an error they are added back for sending later. With waitsForConnectivity = true, the session will not error when there is no connectivity, and signals will instead be lost if an app is quit before regaining connectivity. I guess there's also the chance that someone will quit the app in the middle of the URL request, also causing signals to be lost, though that is less likely (but definitely not impossible).
It might make more sense to revert the change for 'waitsForConnectivity' in favour of a Reachability based solution - this would allow us to receive a notification when an internet connection is restored and at that point we make a fresh attempt to pop items off the cache and post them to the server.
NWPathMonitor (https://developer.apple.com/documentation/network/nwpathmonitor) supports almost all the same versions as this library and is Apple's modern built-in solution.
This sounds like a good idea. Originally I wanted to not do this because I was afraid it would block the SDK from running Linux based apps, but we do have a separate SDK for Vapor now, so that's no longer an issue.