Swift Combine chaining .mapError()

Question

I'm trying to achieve something similar to scenario presented below (create URL, request to server, decode json, error on every step wrapped in custom NetworkError enum):

enum NetworkError: Error {
    case badUrl
    case noData
    case request(underlyingError: Error)
    case unableToDecode(underlyingError: Error)
}

//...
    func searchRepos(with query: String, success: @escaping (ReposList) -> Void, failure: @escaping (NetworkError) -> Void) {
        guard let url = URL(string: searchUrl + query) else {
            failure(.badUrl)
            return
        }

        session.dataTask(with: url) { data, response, error in
            guard let data = data else {
                failure(.noData)
                return
            }

            if let error = error {
                failure(.request(underlyingError: error))
                return
            }

            do {
                let repos = try JSONDecoder().decode(ReposList.self, from: data)

                DispatchQueue.main.async {
                    success(repos)
                }
            } catch {
                failure(.unableToDecode(underlyingError: error))
            }
        }.resume()
    }

My solution in Combine works:

    func searchRepos(with query: String) -> AnyPublisher<ReposList, NetworkError> {
        guard let url = URL(string: searchUrl + query) else {
            return Fail(error: .badUrl).eraseToAnyPublisher()
        }

        return session.dataTaskPublisher(for: url)
            .mapError { NetworkError.request(underlyingError: $0) }
            .map { $0.data }
            .decode(type: ReposList.self, decoder: JSONDecoder())
            .mapError { $0 as? NetworkError ?? .unableToDecode(underlyingError: $0) }
            .subscribe(on: DispatchQueue.global())
            .receive(on: DispatchQueue.main)
            .eraseToAnyPublisher()
    }

but I really don't like this line

.mapError { $0 as? NetworkError ?? .unableToDecode(underlyingError: $0) }

My questions:

1. Is there better way to map errors (and replace line above) using chaining in Combine?
2. Is there any way to include first guard let with Fail(error:) in chain?

Answer 1

I agree with iamtimmo that you don't need .subscribe(on:). I also think this method is the wrong place for .receive(on:), because nothing in the method requires the main thread. If you have code elsewhere that subscribes to this publisher and wants results on the main thread, then that is where you should use the receive(on:) operator. I'm going to omit both .subscribe(on:) and .receive(on:) in this answer.

Anyway, let's address your questions.

1. Is there better way to map errors (and replace line above) using chaining in Combine?

“Better” is subjective. The problem you're trying to solve here is that you only want to apply that mapError to an error produced by the decode(type:decoder:) operator. You can do that using the flatMap operator to create a mini-pipeline inside the full pipeline:

return session.dataTaskPublisher(for: url)
    .mapError { NetworkError.request(underlyingError: $0) }
    .map { $0.data }
    .flatMap {
        Just($0)
            .decode(type: ReposList.self, decoder: JSONDecoder())
            .mapError { .unableToDecode(underlyingError: $0) } }
    .eraseToAnyPublisher()

Is this “better”? Meh.

You could extract the mini-pipeline into a new version of decode:

extension Publisher {
    func decode<Item, Coder>(type: Item.Type, decoder: Coder, errorTransform: @escaping (Error) -> Failure) -> Publishers.FlatMap<Publishers.MapError<Publishers.Decode<Just<Self.Output>, Item, Coder>, Self.Failure>, Self> where Item : Decodable, Coder : TopLevelDecoder, Self.Output == Coder.Input {
        return flatMap {
            Just($0)
                .decode(type: type, decoder: decoder)
                .mapError { errorTransform($0) }
        }
    }
}

And then use it like this:

return session.dataTaskPublisher(for: url)
    .mapError { NetworkError.request(underlyingError: $0) }
    .map { $0.data }
    .decode(
        type: ReposList.self,
        decoder: JSONDecoder(),
        errorTransform: { .unableToDecode(underlyingError: $0) })
    .eraseToAnyPublisher()

2. Is there any way to include first guard let with Fail(error:) in chain?

Yes, but again it's not clear that doing so is better. In this case, the transformation of query into a URL is not asynchronous, so there's little reason to use Combine. But if you really want to do it, here's a way:

return Just(query)
    .setFailureType(to: NetworkError.self)
    .map { URL(string: searchUrl + $0).map { Result.success($0) } ?? Result.failure(.badUrl) }
    .flatMap { $0.publisher }
    .flatMap {
        session.dataTaskPublisher(for: $0)
        .mapError { .request(underlyingError: $0) } }
    .map { $0.data }
    .decode(
        type: ReposList.self,
        decoder: JSONDecoder(),
        errorTransform: { .unableToDecode(underlyingError: $0) })
    .eraseToAnyPublisher()

This is convoluted because Combine doesn't have any operators that can turn a normal output or completion into a typed failure. It has tryMap and similar, but those all produce a Failure type of Error instead of anything more specific.

We can write an operator that turns an empty stream into a specific error:

extension Publisher where Failure == Never {
    func replaceEmpty<NewFailure: Error>(withFailure failure: NewFailure) -> Publishers.FlatMap<Result<Self.Output, NewFailure>.Publisher, Publishers.ReplaceEmpty<Publishers.Map<Publishers.SetFailureType<Self, NewFailure>, Result<Self.Output, NewFailure>>>> {
        return self
            .setFailureType(to: NewFailure.self)
            .map { Result<Output, NewFailure>.success($0) }
            .replaceEmpty(with: Result<Output, NewFailure>.failure(failure))
            .flatMap { $0.publisher }
    }
}

Now we can use compactMap instead of map to turn query into a URL, producing an empty stream if we can't create a URL, and use our new operator to replace the empty stream with the .badUrl error:

return Just(query)
    .compactMap { URL(string: searchUrl + $0) }
    .replaceEmpty(withFailure: .badUrl)
    .flatMap {
        session.dataTaskPublisher(for: $0)
        .mapError { .request(underlyingError: $0) } }
    .map { $0.data }
    .decode(
        type: ReposList.self,
        decoder: JSONDecoder(),
        errorTransform: { .unableToDecode(underlyingError: $0) })
    .eraseToAnyPublisher()

Answer 2

I don't think your approach is unreasonable. A benefit of the first mapError() (at // 1) is that you don't need to know much about the possible errors from the request.

    return session.dataTaskPublisher(for: url)
        .mapError { NetworkError.request(underlyingError: $0) }   // 1
        .map { $0.data }
        .decode(type: ReposList.self, decoder: JSONDecoder())
        .mapError { $0 as? NetworkError ?? .unableToDecode(underlyingError: $0) }
        .subscribe(on: DispatchQueue.global())   // 2 - not needed
        .receive(on: DispatchQueue.main)
        .eraseToAnyPublisher()
    }

I don't think you need the subscribe(on:) at // 2, since URLSession.DataTaskPublisher starts on a background thread already. The subsequent receive(on:) is required.

An alternative approach would be to run through the "happy path" first and map all of the errors later, as in the following. You'll need to understand which errors come from which publishers/operators to correctly map to your NetworkError enum.

    return session.dataTaskPublisher(for: url)
        .map { $0.data }
        .decode(type: ReposList.self, decoder: JSONDecoder())
        .mapError({ error -> NetworkError in
            // map all the errors here
        })
        .receive(on: DispatchQueue.main)
        .eraseToAnyPublisher()

To handle your second question, you can use the tryMap() and flatMap() to map your query into a URL and then into a URLSession.DataTaskPublisher instance. I haven't tested this particular code, but a solution would be along these lines.

    Just(query)
        .tryMap({ query in
            guard let url = URL(string: searchUrl + query) else { throw NetworkError.badUrl }
            return url
        })
        .flatMap({ url in
            URLSession.shared.dataTaskPublisher(for: url)
                .mapError { $0 as Error }
        })
        .map { $0.data }
        //
        // ... operators from the previous examples
        //
        .eraseToPublisher()