feat: add iOS background request keep-alive #68
Conversation
|
Thank you @tcldr for the PR. I will try to review it this week. |
There was a problem hiding this comment.
Thanks @tcldr. See comments/questions.
Also maybe the beginBackgroundTask logic should be applied only for background flush() (invoked in exitForeground) not for all uploads. What do you think?
| // add some throughput | ||
| sessionConfiguration.httpMaximumConnectionsPerHost = 2 | ||
| // avoid keeping app background task alive for too long | ||
| sessionConfiguration.timeoutIntervalForRequest = 20 |
There was a problem hiding this comment.
This will limit all requests to 20s, vs default of 60s which could increase timeouts in low latency situations. I would prefer to leave this as the default.
Also should we use timeoutIntervalForResource be used instead of timeoutIntervalForRequest?
There was a problem hiding this comment.
timeoutIntervalForRequest? is the maximum duration a task should wait for additional data to arrive before giving up. In other words, it will cancel a partially uploaded task if it's been greater than the timeout interval since the last packet was sent. The timer is reset each time the task progresses.
the default timeoutIntervalForResource is 7 days, and is the maximum duration a task should take from initiation to completion. It's more relevant for long running downloads or uploads. I think the default is fine here.
In terms of whether it should be 20s or 60s, 20s has the benefit of being under the background task deadline time (30s) so helps avoids the app being penalised by the system for running for too long. Also, as flush() is automatically being called every 30s or so, any timeout task will soon be retried. This isn't a huge deal though – I'm happy with 60s if you're more comfortable with it.
There was a problem hiding this comment.
Thanks for the detailed explanation. We would prefer to keep the default timeout.
|
Hi @justin-fiedler , left some comments, let me know your thoughts and I can update the PR. Cheers! |
|
Superseded by #78. |
Summary
When an iOS app is backgrounded, any network requests which aren't specifically marked as background tasks will fail to complete. With the current implementation, this means that up to 30 seconds of events will be delayed until the next time the app is opened, or lost completely.
iOS does include an affordance to extend the time after an app loses foreground focus in which it can perform additional tasks, but these tasks need to be marked explicitly with calls to
UIApplication.beginBackgroundTaskandUIApplication.endBackgroundTask.Unfortunately, this can't be achieved with an event callback alone as a background task should ideally be limited to around 5s, with a maximum deadline of 30s. This deadline is frequently exceeded with an event callback as the duration between when an event is tracked and when it is finally flushed through the queue frequently exceeds 30s and results in warnings from the system.
The PR works around this issue by calling the system's
UIApplication.beginBackgroundTaskandUIApplication.endBackgroundTaskmethods as part of the EventPipeline mechanism, indicating to the system that it should keep the app alive until the point that all pending network requests have completed. It also includes some changes to the setup ofURLSessionto increase the chances of a successful network request being made, and to avoid background tasks running over deadline.An alternative way to achieve this might be to include an additional callback in the options that gets fired just prior to dispatch, but on balance this seems a more pragmatic approach that doesn't require the addition of any public API.
Checklist