Feature Request: API for cancelling underlying async task

[loudmouth]

I am wrapping URLSessionDataTask instances in Observable instances via code similar to that listed below (I have removed the use of generics and replaced them with concrete types to, hopefully, make the example more clear).

// Where the conversion of the datatask to the signal occurs.
func toObservable(url: URL, dataTaskToConvert: (URL, (Result<Data>) -> Void)) -> Observable<Result<Data>> {

    let signal = Observable<Result<Data>>()

    let value: URLSessionDataTask? = dataTaskToConvert(url) { result in
        signal.update(result)
    }
    return signal
}

// Example API for consumer to make network requests without ugly callbacks.
func fetch(url: URL) ->  Observable<Result<Data>> {
    return toObservable(url: url, dataTaskToConvert: fetch)
}

// DataTask response handling is transformed to use an API with Interstellar's Result type like so:
func fetch(url: URL, completion: @escaping (Result<Data>) -> Void) -> URLSessionDataTask {
    let task = sharedUrlSession.dataTask(with: url) { data, response, error in
        if let data = data {
            completion(Result.success(data))
            return
        }

        if let error = error {
            completion(Result.error(error))
            return
        }

        let sdkError = SDKError.invalidHTTPResponse(response: response)
        completion(Result.error(sdkError))
    }

    task.resume()
    return task
}

Unfortunately, I haven't figured out a way to expose the underlying URLSessionDataTask to the client to cancel the request in a clean fashion. I think ideally, there would be somesort of method on the observable to stop the signal. My workaround currently is to return tuple of (URLSessionDataTask, Observable<Result<Data>>) rather than just the Observable so that the client can take advantage of the Observable API provided by Interstellar while also having the ability to cancel the request. However, I find the tuple API ugly to use, as client code would look like the following:

// must access the tuple element at index `1` (or named parameter)
fetch(url: someURL).1.then { data in
 // do stuff with data
}.error { 
    // handle error.
}

I am relatively new to the reactive style of programming, so I am currently unable to figure out what a good strategy for disposing the observable and it's underlying might be. I looked into the section on Disposables in the README for ReactiveKit, but I'm not sure how the same concept would integrate with Interstellar.

Is there a way you might recommend cancelling the URLSessionDataTask? I would be very happy to work on a pull request (with a small bit of guidance to kick me off in the right direction) if you think this feature might be useful.

[JensRavens]

I would recommend you to use a similar style to disposables.

Interstellar uses a slightly different memory management concept: Instead of explicit disposables it's just plain ARC. As soon as there are no reference to an observable it will just be deallocated, taking all it's data with it. You can use this by observing some wrapper class around your data that cancels the request on deinit:

class Request {
  var task: URLSessionDataTask

  deinit {
    task.cancel()
  }
}

let request = ...
var observable: Observable<Request>? = Observable(request) // request is still running
obserable = nil // effectively canceling the request

[loudmouth]

Thanks @JensRavens. I will revisit this next week.

Closing for now.

[loudmouth]

Hey @JensRavens, I looked into it this morning and one thing I'd like to do is preserve the type signature of my Observable. In this case, the type is Observable<Result<DataType>> where DataType is the expected type after the network request returns successfully, and mapping from has occurred. This is in contrast to the above-suggested Observable<Request> type signature.

As an experiment, I checked out of a branch of Interstellar and simply added the following to Observable:

var cancellationBlock: ((Void) -> Void)?

deinit {
  cancellationBlock?()
}

And then of course I defined

observable.cancellationBlock = {
   task?.cancel()
}

This indeed enables the cancellation any underlying task by simply releasing the observable as suggested. I'm curious what you think of this approach and if a PR would be welcomed.

I would like to avoid creating a wrapper class around Observable as it would clutter the API provided by my library as consumers would either have to access Observable methods via myRequest.observable or this wrapper class would have to forward all methods of Observable.

[loudmouth]

Hey @JensRavens , I now understand your suggestion a bit better after hacking for half a day and continuing to look at more resources about programming with Observables. I was happily surprised to find out that I didn't need to forward all methods when making a wrapper class to enable cancellation—rather that I could transform an Observable<Request> to an Observable<Result<ResultType>>, and then simply nullify my Observable<Result<ResultType>> instance to cancel the request.

Pasting the code here in case other's who are doing networking with Interstellar find this issue.

internal class Request {
    var task: URLSessionDataTask?

    init(task: URLSessionDataTask?) {
        self.task = task
    }

    deinit {
        print("cancelling request")
        task?.cancel()
    }
}

// Where the conversion of the datatask to the signal occurs.
func toObservable(url: URL, dataTaskToConvert: (URL, (Result<Data>) -> Void)) -> Observable<Result<Data>> {

    let dataObservable = Observable<Result<Data>>()

    let task: URLSessionDataTask? = dataTaskToConvert(url) { result in
        dataObservable.update(result)
    }
    let observable = Observable<Request>(Request(task: task))
    // Tranfrom from Observable<Request> to Observable<Data>
    return observable.flatMap { _ -> Observable<Result<Data>> in
        return responseObservable
    }
}

Thanks again.

[loudmouth]

I may have spoken too soon. My implementation above immediate releases the Observable<Request> which contains the URLSessionTask of interest as soon as the function exits.

I'm feeling like either adding a cancellationBlock to Observable as suggested above, or some other implementation of Disposable is the way forward.