'fetchDecodable'-function does not immediately complete in case of an error

[chrishoste]

• contentful.swift version number: 5.5.11
• Xcode version number: 15.3.0
• Target operating system(s) and version number(s)
  • iOS: (But prob. also an issue on other OS)
  • tvOS:
  • watchOS:
  • macOS:
• Package manager: Swift Package Manager

---

First, let's have a look at the code:

internal func fetchDecodable<DecodableType: Decodable>(
    url: URL,
    completion: @escaping ResultsHandler<DecodableType>
) -> URLSessionDataTask {
    let finishDataFetch: (ResultsHandler<Data>) = { result in
        switch result {
        case .success(let mappableData):
            self.handleJSON(data: mappableData, completion: completion)
        case .failure(let error):
            completion(.failure(error))
        }
    }

    let task = fetchData(url: url) { dataResult in
        if url.lastPathComponent == "locales" || url.lastPathComponent == self.spaceId {
            // Now that we have all the locale information, start callback chain.
            finishDataFetch(dataResult)
        } else {
            self.fetchLocalesIfNecessary { localesResult in
                switch localesResult {
                case .success:
                    // Trigger chain with data we're currently interested in.
                    finishDataFetch(dataResult)
                case .failure(let error):
                    // Return the current error.
                    finishDataFetch(.failure(error))
                }
            }
        }
    }
    return task
}

Especially this part:

let task = fetchData(url: url) { dataResult in
    if url.lastPathComponent == "locales" || url.lastPathComponent == self.spaceId {
        // Now that we have all the locale information, start callback chain.
        finishDataFetch(dataResult)
    } else {
        self.fetchLocalesIfNecessary { localesResult in
            switch localesResult {
            case .success:
                // Trigger chain with data we're currently interested in.
                finishDataFetch(dataResult)
            case .failure(let error):
                // Return the current error.
                finishDataFetch(.failure(error))
            }
        }
    }
}

What does fetchData do? It takes in a URL and opens up a dataTask for it, trying to retrieve data for the specific URL. In any case, the completion block of the dataTask is invoked. So far so good.

Now let's say we have no internet connection, then fetchData will complete with an error. So the dataResult in the code should be a .failure with an error. But this is never checked in the code above. So if the URL does not comply with the condition, the code will then jump into another fetch. In this case, the fetchLocalesIfNecessary function logically, if you have no internet connection, will complete with a failure too. Now it's checked, and fetchDecodable will complete with an error.

In the success case of the second fetch, the dataResult of fetchData is sent anyway, which is a failure. This means it will fail in the next step.

Now, in case of a no internet connection error, this is not a big deal because the errors are sent immediately by the URLSession. But let's say this will time out (URLSessionConfiguration default timeout of 60s), then fetchDecodable will timeout in 120s! In this time, I could already have retried once and would be about to trigger my second retry.

So either we wrap this code into a switch and also check for a failure:

let task = fetchData(url: url) { dataResult in
    switch dataResult {
    case .success:
        if url.lastPathComponent == "locales" || url.lastPathComponent == self.spaceId {
            // Now that we have all the locale information, start callback chain.
            finishDataFetch(dataResult)
        } else {
            // ....
        }
    case .failure:
        // Immediately complete with the failure case
        finishDataFetch(dataResult)
    }
}

Or add any guard or if let right before it:

let task = fetchData(url: url) { dataResult in
    guard let data = try? dataResult.get() else {
        // Handle the failure case if unable to get data
        // Immediately complete with the failure case
        finishDataFetch(dataResult)
        return
    }

    // Proceed with the logic if data retrieval was successful
    if url.lastPathComponent == "locales" || url.lastPathComponent == self.spaceId {
        // Now that we have all the locale information, start callback chain.
        finishDataFetch(dataResult)
    } else {
       // ...
    }
}

This would handle every error occurring in fetchData immediately and avoid unnecessary fetch requests. If you need any help or if I am completely wrong, or if this is expected behavior, just let me know. I can also open up a PR if you want.

Here you can also see that it is logged two times in a row: