URLSession.shared.dataTask correct way to receive data

Question

Good day!

I am a little confused in trying to find the correct sequence in checking received (data, response, error) from dataTask and doing some special error handling.

Usually we have URLSession looking like this:

class HTTPRequest {
    static func request(urlStr: String, parameters: [String: String], completion: @escaping (_ data: Data?,_ response: URLResponse?, _ error: Error?) -> ()) {
        var url = OpenExchange.base_URL + urlStr
        url += getParameters(param: parameters)
        let request = URLRequest(url: URL(string: url)!)
        let task = URLSession.shared.dataTask(with: request) { (data, response, error) in
            if error != nil {
                print("URLSession Error: \(String(describing: error?.localizedDescription))")
                completion(nil,nil,error)
            } else {
                completion(data,response,nil)
            }
        }
        task.resume()
    }

    static func getParameters(param: [String: String]) -> String {
        var data = [String]()
        for (key,value) in param {
            data.append(key + "=\(value)")
        }
        return data.map { String($0) }.joined(separator: "&")
    }

}

I have another function that has HTTPRequest inside of it, to wrap everything to and object type I'm working with:

 static func networkOperation(urlStr: String, parameters: [String: String], completion: @escaping (ReturnedData) -> () ) {
        var recieved = ReturnedData()
        HTTPRequest.request(urlStr: urlStr, parameters: parameters) { (data, resp, err) in
            if let data = data, let response = resp {

// TODO: try JSONDecoder() if data is API Error Struct; Moderate this section depending on results of decoding;

                recieved.data = data
                recieved.response = response 
                recieved.result = .Success
                completion(recieved)
                return
            } else if err == nil {
                recieved.result = .ErrorUnknown
                completion(recieved)
                return
            }
            recieved.error = err as NSError?
            completion(recieved)
        }
       }

public struct ReturnedData {
    public var data: Data?
    public var response: URLResponse?
    public var error: Error?
    public var result: RequestResult = .ErrorHTTP
}

public enum RequestResult: String {
    case Success
    case ErrorAPI
    case ErrorHTTP
    case ErrorUnknown
}

Using the code above I can easily create different networkOperation calls for doing different API methods and handle different Data models that are returned. What I am trying to implement is API Error check. Since my API has some error description for example when you get your APP_ID wrong or current APP_ID has no permission to get information etc.. So if any of these occur the data will look like this:

  {
  "error": true,
  "status": 401,
  "message": "invalid_app_id",
  "description": "Invalid App ID provided - please sign up at https://openexchangerates.org/signup, or contact [email protected]."
  }

I think its not alright trying to decode every received data with Error struct in networkOperations "//TODO" mark, maybe there is some good way to implement this?

Answer 1

You should have your API errors return error objects.

E.g. You could do:

enum NetworkRequestError: Error {
    case api(_ status: Int, _ code: ApiResultCode, _ description: String)
}

Where you code your responses into an enum called ApiResultCode like so:

enum ApiResultCode {
    case invalidAppId
    case recordNotFound   // just an example
    ...
    case unknown(String)
}

extension ApiResultCode {
    static func code(for string: String) -> ApiResultCode {
        switch string {
        case "invalid_app_id":   return .invalidAppId
        case "record_not_found": return .recordNotFound
        ...
        default:                 return .unknown(string)
        }
    }
}

This enum lets you check message codes without littering your code with string literals.

And if you parse an API error, you could return that. E.g.

if responseObject.error {
    let error = NetworkRequestError.api(responseObject.status, ApiResultCode.code(for: responseObject.message), responseObject.description)
    ... now pass this `error`, just like any other `Error` object
}

---

If you’re open to a broader redesign, I’d personally suggest

• refactor RequestResult to pull out those individual error types (the caller wants to know simply if it succeeded or failed ... if it failed, it should then look at the Error object to determine why it failed);
• but have this new Result enumeration include associated values, namely the Data on success and the Error on failure; and
• now that the enumeration includes what we need in its associated values, we can completely eliminate ReturnedData.

So, first, let’s expand that RequestResult to include the error on failures and the payload on success:

public enum Result {
    case success(Data)
    case failure(Error)
}

Actually, modern convention is to make this generic, where the above becomes a Result<Data, Error> using the following:

public enum Result<T, U> {
    case success(T)
    case failure(U)
}

(Swift 5 actually includes this generic.)

And I’d then expand ResultError to handle both API errors as well as any unknown errors:

enum NetworkRequestError: Error {
    case api(_ status: Int, _ code: ApiResultCode, _ description: String)
    case unknown(Data?, URLResponse?)
}

So, having done this, you can change request to pass back a Result<Data, Error>:

static func request(urlString: String, parameters: [String: String], completion: @escaping (Result<Data, Error>) -> ()) {
    let request = URLRequest(url: URL(string: urlString)!)
    let task = URLSession.shared.dataTask(with: request) { data, response, error in
        guard let responseData = data, error == nil else {
            completion(.failure(error ?? NetworkRequestError.unknown(data, response)))
            return
        }

        completion(.success(responseData))
    }
    task.resume()
}

And the caller would then do:

request(...) { result in
    switch result {
    case .failure(let error):
        // do something with `error`

    case .success(let data):
        // do something with `data`
    }
}

The beauty of this Result generic is that it becomes a consistent pattern you can use throughout your code. For example, let’s assume you have some method that is going to parse a Foo object out of the Data that request returned:

func retrieveFoo(completion: @escaping (Result<Foo, Error>) -> Void) {
    request(...) { result in
        switch result {
        case .failure(let error):
            completion(.failure(error))

        case .success(let data):
            do {
                let responseObject = try JSONDecoder().decode(ResponseObject.self, from: data)
                if responseObject.error {
                    completion(.failure(NetworkRequestError.api(responseObject.status, ApiResultCode.code(for: responseObject.message), responseObject.description)))
                    return
                }

                let foo = responseObject.foo
                completion(.success(foo))
            } catch {
                completion(.failure(error))
            }
        }
    }
}

Or, if you wanted to test for a particular API error, e.g. .recordNotFound:

retrieveFoo { result in
    switch result {
    case .failure(NetworkRequestError.api(_, .recordNotFound, _)):
        // handle specific “record not found” error here

    case .failure(let error):
        // handle all other errors here

    case .success(let foo):
        // do something with `foo`
    }
}