Alamofire: Background download restore crashes Alamofire 5

[ralcr]

• [ x] I've read, understood, and done my best to follow the *CONTRIBUTING guidelines.

What did you do?

Trying to resume an unfinished background download by recreating the session and listening for taskDidComplete EventMonitor

What did you expect to happen?

I expect to receive an error with resume data

What happened instead?

Crashes in RequestTaskMap: disassociateIfNecessaryAfterGatheringMetricsForTask and disassociateIfNecessaryAfterCompletingTask. The fix is to return early instead crash because i didn't started the download and there's no task in the dictionary.
Also, as a side note, after the resume download finishes it is not removed from the temporary folder.

Alamofire Environment

Alamofire version:5 beta 6
Xcode version:10.2
Swift version:5
Platform(s) running Alamofire:iOS
macOS version running Xcode:Mojave

Demo Project

https://github.com/ralcr/BackgroundDownload_iOS/blob/master/BackgroundDownload/DownloadAlamofire.swift

[jshier]

As mentioned on Twitter, Alamofire doesn't officially support background sessions and their resume requirements, as we aren't able to persist the state we need while the app isn't running.

That said, it is interesting (and news to me) that URLSessions with the same background identifier will immediately start receiving events. @ralcr Does this issue still occur if you don't implement application(_:handleEventsForBackgroundURLSession:completionHandler:)? I wonder if the reconnection behavior is dependent on that.

@cnoon In the long run, until we have better background support, we may want to do something to indicate that we don't support this type of usage. What do you think?

[tony-osibov]

thanks @ralcr for posting the issue report. i can confirm i am having this same issue as well. it started as soon as i began using a background session.

this may be easier said than done, but it would seem to me since af doesn't officially support background sessions that the existing code written to enable it should be made open so that for those of us who want to take advantage can prevent the fatal errors from occurring or expand upon it further. as it stands right now the properties and methods required to fix this issue are all internal to the module which means the only solution until a mainline fix is to fork the code.

@jshier if this is something that you and the team would be open to i am happy to submit a pr with my proposed open changes.

[jshier]

@tony-osibov What exactly did you have in mind? We'd rather not open up the classes at the moment, due to the complexity of properly subclassing, but if you have a specific fix in mind that we could integrate, let us know.

[ralcr]

Hi @jshier , the events will still be fired even without handleEventsForBackgroundURLSession method.
At the moment i reimplemented the downloads with URLSession, but being a large project probably that's how will stay forever even if you add later background download.
In my opinion, even if a proper design to support background download is not possible now, you should remove the crash and add the handleEventsForBackgroundURLSession as an event, not sure how hard it is, should be called by func urlSessionDidFinishEvents (forBackgroundURLSession session: URLSession). I'd be happy to contribute with a sample code on how to do background download on your repo then. Alamofire4 didn't fully supported it either but was possible.

[jshier]

@cnoon Any thoughts here? I'd rather not lessen the requirements of the task map, but perhaps we conditionally do so if we're configured for a background session.

[cnoon]

Oh I have some thoughts @jshier... 😉

First off everyone, thank you for pitching in here to help make things work better for background sessions. It's awesome to see the community rally around these types of things. This one, unfortunately, is much MUCH bigger than this thread is hinting at. Proper background support requires us to move completely away from any closure based APIs since they go out-of-memory once the app is terminated (which happens all the time with background sessions) for proper background session support.

To properly support background sessions (something absolutely in our backlog), we'll need to create a second type of Session that doesn't use Request chaining APIs and closures and is designed solely to support background sessions. This will ensure that you can safely tear down the entire app, set your background session type back up when your app is relaunched with a matching identifier, and then handle the completions correctly since we didn't throw all your completion closures away when the app terminated. As I'm sure you can see, this is a BIG effort, and something we've wanted to do for a very long time.

I've created a similar system in an internal library we use at Nike, but I haven't had time yet to start to move this type of functionality into AF. It is, however, the next big thing I'd like to contribute back to AF.

In the meantime, I'd recommend we consider updating the documentation to state explicitly that we do not support background sessions, as well as potentially going even a step further adding an assertion for background sessions since they really don't work if your app gets terminated and restarted. If we do do this, then I think I'd also recommend we add a blurb in the documentation explaining what I explained above as what we're thinking for future support.

Thoughts @jshier and group?

[jshier]

We've merged changes to make the creations of an Alamofire Session using a background URLSessionConfiguration or a URLSession already configured for the background a fatal error at runtime. While we applaud the industriousness of everyone who figured out a way to make this work to some degree, we'd prefer to develop a real solution that than surprising everyone with an internal consistency error.