Mapping of BothamAPIError is done in NSHTTPClient and HTTPClient

[davideme]

We should review this code and maybe unify it inside NSHTTPClient.

[pedrovgs]

I don't understand you. Right now the implementation is just in HTTPClient using an extension.

extension HTTPClient {

    public var timeout: NSTimeInterval {
        get {
            return 10
        }
    }

    public func hasValidScheme(request: HTTPRequest) -> Bool {
        return request.url.hasPrefix("http") || request.url.hasPrefix("https")
    }

    public func isValidResponse(response: HTTPResponse) -> Bool {
        return 200..<300 ~= response.statusCode
    }

    public func mapNSErrorToBothamError(error: NSError) -> BothamAPIClientError {
        let connectionErrors = [NSURLErrorCancelled,
            NSURLErrorTimedOut,
            NSURLErrorCannotConnectToHost,
            NSURLErrorNetworkConnectionLost,
            NSURLErrorNotConnectedToInternet,
            NSURLErrorRequestBodyStreamExhausted
        ]
        if connectionErrors.contains(error.code) {
            return .NetworkError
        } else {
            return .HTTPClientError(error: error)
        }
    }

}

And NSHTTPClient uses this method here:

 public func send(httpRequest: HTTPRequest, completion: (Result<HTTPResponse, BothamAPIClientError>) -> ()) {
        let request = mapHTTPRequestToNSURLRequest(httpRequest)
        let session = NSURLSession.sharedSession()
        session.configuration.timeoutIntervalForRequest = timeout
        session.configuration.timeoutIntervalForResource = timeout
        session.dataTaskWithRequest(request) { data, response, error in
            if let error = error {
                let bothamError = self.mapNSErrorToBothamError(error)
                completion(Result.Failure(bothamError))
            } else if let response = response as? NSHTTPURLResponse, let data = data {
                let response = self.mapNSHTTPURlResponseToHTTPResponse(response, data: data)
                completion(Result.Success(response))
            }
        }.resume()
    }

[davideme]

Sorry my bad.
I meant BothamAPIClient and NSHTTPClient/HTTPClient.

///HTTPClient/NSHTTPClient
    public func mapNSErrorToBothamError(error: NSError) -> BothamAPIClientError {
        let connectionErrors = [NSURLErrorCancelled,
            NSURLErrorTimedOut,
            NSURLErrorCannotConnectToHost,
            NSURLErrorNetworkConnectionLost,
            NSURLErrorNotConnectedToInternet,
            NSURLErrorRequestBodyStreamExhausted
        ]
        if connectionErrors.contains(error.code) {
            return .NetworkError
        } else {
            return .HTTPClientError(error: error)
        }
    }

//BothamAPIClient
    private func mapHTTPResponseToBothamAPIClientError(httpResponse: HTTPResponse)
        -> Result<HTTPResponse, BothamAPIClientError> {
            if isValidResponse(httpResponse) {
                return Result.Success(httpResponse)
            } else {
                let statusCode = httpResponse.statusCode
                let body = httpResponse.body
                return Result.Failure(.HTTPResponseError(statusCode: statusCode, body: body))
            }
    }

[pedrovgs]

I'm not sure about this one. Inside an HTTPClient implementation a response with a 401 is not an error. I mean, the request was sent to the server and the server responded with an error. But, we've changed HTTPClient api to use BothamAPIClientError. I'm going to prepare a PR with the change, so we can discuss with some code :)

[davideme]

No, I don't want a PR just yet. Just to discuss if it make sense to have
this 2 layer.

On Thu, Jan 7, 2016 at 11:32 AM, Pedro Vicente Gómez Sánchez <
[email protected]> wrote:

I'm not sure about this one. Inside an HTTPClient implementation a
response with a 401 is not an error. I mean, the request was sent to the
server and the server responded with an error. But, we've changed
HTTPClient api to use BothamAPIClientError. I'm going to prepare a PR
with the change, so we can discuss with some code :)

—
Reply to this email directly or view it on GitHub
#30 (comment)
.