Improve error messages and recovery suggestions

Resolves #35 `Improve Error Explanation for Rate Limit`.

- Adds checks for rate limiting and uses new, specific error
- Refactors response validation to static methods in an extension of
`ResponseHandler`, thus providing a default implementation to all
response handlers
- Refactors the deserialisation of JSON in `JSONResponseHandler` into a
separate method to improve readability
- Updates `GitHubScannerProtocolError` to inherit from `LocalizedError`
and provides a default implementation for `CustomStringConvertible`
- Adds `errorDescription` and `recoverySuggestion` to all error types
- Moves `NetworkError` and `ScanOptionsError` into own files
- Expands test suite for error handling
- Refactors accessing the `HTTPURLResponse` next link header into an
extension property to improve readability of `JSONResponseHandler` and
to help maintain single responsibility principle

Sources/GitHubKit/Extension/HTTPURLResponse+Additions.swift

@@ -3,7 +3,7 @@
 //  GitHub Scanner
 //
 //  Created by Aaron McTavish on 10/02/2017.
-//  Copyright © 2016 ustwo Fampany Ltd. All rights reserved.
+//  Copyright © 2017 ustwo Fampany Ltd. All rights reserved.
 //
 
 import Foundation
@@ -14,6 +14,17 @@ import Foundation
 extension HTTPURLResponse {
 
 
+    // MARK: - Types
+
+    private struct Constants {
+        static let nextLinkHeader = "next"
+        static let urlHeaderValue = "url"
+    }
+
+
+    // MARK: - Properties
+
+    /// Parses the link headers from the `HTTPURLResponse` and returns a dictionary.
     public var links: [String: [String: String]] {
         var result = [String: [String: String]]()
 
@@ -57,4 +68,9 @@ extension HTTPURLResponse {
         return result
     }
 
+    /// Returns the 'next' link header, if it exists.
+    public var nextLink: String? {
+        return links[Constants.nextLinkHeader]?[Constants.urlHeaderValue]
+    }
+
 }

Sources/GitHubKit/Networking/NetworkClient.swift

@@ -3,7 +3,7 @@
 //  GitHub Scanner
 //
 //  Created by Aaron McTavish on 09/02/2017.
-//  Copyright © 2016 ustwo Fampany Ltd. All rights reserved.
+//  Copyright © 2017 ustwo Fampany Ltd. All rights reserved.
 //
 
 
@@ -12,13 +12,6 @@
 import Foundation
 
 
-public enum NetworkError: Error {
-    case failedRequest(status: Int)
-    case invalidJSON
-    case unknown(error: Error?)
-}
-
-
 public final class NetworkClient {
 
 

Sources/GitHubKit/Networking/NetworkError.swift

@@ -0,0 +1,55 @@
+//
+//  NetworkError.swift
+//  GitHub Scanner
+//
+//  Created by Aaron McTavish on 20/02/2017.
+//  Copyright © 2017 ustwo Fampany Ltd. All rights reserved.
+//
+
+
+import Foundation
+
+
+public enum NetworkError: LocalizedError {
+    case failedRequest(status: Int)
+    case invalidJSON
+    case rateLimited
+    case unauthorized
+    case unknown(error: Error?)
+}
+
+
+extension NetworkError {
+
+
+    public var errorDescription: String? {
+        switch self {
+        case let .failedRequest(status):
+            return "Failed Request. Status Code: \(status)"
+        case .invalidJSON:
+            return "Invalid JSON returned from the server"
+        case .rateLimited:
+            return "Exceed rate limit for requests"
+        case .unauthorized:
+            return "Not authorized"
+        case let .unknown(error):
+                if let localError = error as? LocalizedError,
+                let description = localError.errorDescription {
+
+                return "Unknown Error: " + description
+            } else {
+                return "Unknown Error"
+            }
+        }
+    }
+
+    public var recoverySuggestion: String? {
+        switch self {
+        case .rateLimited, .unauthorized:
+            return "Use the '--oauth' flag and supply an access token"
+        case .failedRequest, .invalidJSON, .unknown:
+            return nil
+        }
+    }
+
+}

Sources/GitHubKit/Networking/ResponseHandler.swift

@@ -3,13 +3,22 @@
 //  GitHub Scanner
 //
 //  Created by Aaron McTavish on 10/02/2017.
-//  Copyright © 2016 ustwo Fampany Ltd. All rights reserved.
+//  Copyright © 2017 ustwo Fampany Ltd. All rights reserved.
 //
 
 
 // swiftlint:disable large_tuple
 
 import Foundation
+import Result
+
+
+public typealias NetworkValidationResult = Result<(Data, HTTPURLResponse), NetworkError>
+
+
+public enum ResponseHandlers {
+    static let `default` = JSONResponseHandler()
+}
 
 
 public protocol ResponseHandler {
@@ -24,6 +33,73 @@ public protocol ResponseHandler {
 }
 
 
-public enum ResponseHandlers {
-    static let `default` = JSONResponseHandler()
+extension ResponseHandler {
+
+
+    /// Checks the response's status code to ensure it is in the 200 range.
+    ///
+    /// - Parameter response: The `HTTPURLResponse` to validate.
+    /// - Returns: Whether or not the response is valid.
+    public static func isValidResponseStatus(_ response: HTTPURLResponse) -> Bool {
+        return 200..<300 ~= response.statusCode
+    }
+
+    /// Checks the response to see if it failed due to rate limiting.
+    ///
+    /// - Parameter response: The `HTTPURLResponse` to validate.
+    /// - Returns: Whether or not the response has been rate limited.
+    public static func isRateLimitedResponse(_ response: HTTPURLResponse) -> Bool {
+        if [401, 403].contains(response.statusCode),
+            let rateLimitString = response.allHeaderFields["X-RateLimit-Remaining"] as? String,
+            let rateLimit = Int(rateLimitString),
+            rateLimit == 0 {
+
+            return true
+        }
+
+        return false
+    }
+
+    /// Checks the response to see if it failed due to not having sufficient authorization on the request.
+    ///
+    /// - Parameter response: The `HTTPURLResponse` to validate.
+    /// - Returns: Whether or not the response failed due to being unauthorized.
+    ///
+    /// - Note: Should check `ResponseHandler.isRateLimitedResponse(_:)` before calling this method,
+    ///         as being rate limited could also return a 401 status code.
+    public static func isUnauthorized(_ response: HTTPURLResponse) -> Bool {
+        return response.statusCode == 401
+    }
+
+    /// Validates a network response.
+    ///
+    /// - Parameters:
+    ///   - data: `Data` from the body of the response.
+    ///   - response: `URLResponse` to validate.
+    ///   - error: `Error` from making the network request.
+    /// - Returns: If the validation is successful, returns the `data` unwrapped and the response
+    ///             as an `HTTPURLResponse`. If the validation is unsuccessful, returns the `NetworkError`
+    ///             representing the issue.
+    ///
+    /// - Note: Checks for a valid status code, rate limiting error, and authorization.
+    public static func validateResponse(data: Data?, response: URLResponse?, error: Error?) -> NetworkValidationResult {
+        guard let httpResponse = response as? HTTPURLResponse,
+            let data = data else {
+
+                return .failure(NetworkError.unknown(error: error))
+        }
+
+        guard Self.isValidResponseStatus(httpResponse) else {
+            if Self.isRateLimitedResponse(httpResponse) {
+                return .failure(NetworkError.rateLimited)
+            } else if Self.isUnauthorized(httpResponse) {
+                return .failure(NetworkError.unauthorized)
+            }
+
+            return .failure(NetworkError.failedRequest(status: httpResponse.statusCode))
+        }
+
+        return .success((data, httpResponse))
+    }
+
 }

Sources/GitHubKit/Networking/ResponseHandlers/JSONResponseHandler.swift

@@ -11,40 +11,57 @@
 
 
 import Foundation
+import Result
 
 
 public final class JSONResponseHandler: ResponseHandler {
 
+
+    // MARK: - ResponseHandler
+
     public func process<Output: JSONInitializable>(data: Data?,
                                                    response: URLResponse?,
                                                    error: Error?,
                                                    completion: ((_ result: Output?,
                                                                  _ linkHeader: String?,
                                                                  _ error: NetworkError?) -> Void)?) {
 
-        guard let httpResponse = response as? HTTPURLResponse,
-            let data = data else {
+        let validationResult = JSONResponseHandler.validateResponse(data: data,
+                                                                    response: response,
+                                                                    error: error)
 
-                completion?(nil, nil, NetworkError.unknown(error: error))
+        switch validationResult {
+        case let .success(body, httpResponse):
+            let deserializationResult: Result<Output, NetworkError> = deserializeJSON(data: body)
+
+            switch deserializationResult {
+            case let .success(result):
+                completion?(result, httpResponse.nextLink, nil)
                 return
-        }
 
-        guard 200..<300 ~= httpResponse.statusCode else {
-            completion?(nil, nil, NetworkError.failedRequest(status: httpResponse.statusCode))
+            case let .failure(deserializationError):
+                completion?(nil, nil, deserializationError)
+                return
+            }
+
+        case let .failure(validationError):
+            completion?(nil, nil, validationError)
             return
         }
+    }
 
-        if let json = try? JSONSerialization.jsonObject(with: data),
-            let result = Output(json: json) {
+    /// Deserializes the JSON into a model, if possible.
+    ///
+    /// - Parameter data: The JSON `Data` to deserialize.
+    /// - Returns: If success, returns the deserialized model. Otherwise, returns a `NetworkError`.
+    func deserializeJSON<Output: JSONInitializable>(data: Data) -> Result<Output, NetworkError> {
+        guard let json = try? JSONSerialization.jsonObject(with: data),
+            let result = Output(json: json) else {
 
-            let links = httpResponse.links
-            let link = links["next"]?["url"]
-            completion?(result, link, nil)
-            return
+            return .failure(NetworkError.invalidJSON)
         }
 
-        completion?(nil, nil, NetworkError.invalidJSON)
-        return
+        return .success(result)
     }
 
 }

Sources/GitHubScannerKit/Command/ScanOptions.swift

@@ -12,38 +12,6 @@ import GitHubKit
 import Result
 
 
-public enum ScanOptionsError: GitHubScannerProtocolError {
-    case invalidCategory(value: String)
-    case invalidRepositoryType(value: String)
-    case missingAuthorization
-    case missingOwner
-    case unknown(error: Error)
-}
-
-
-extension ScanOptionsError: Equatable {
-
-    public static func == (lhs: ScanOptionsError, rhs: ScanOptionsError) -> Bool {
-        switch (lhs, rhs) {
-        case (let .invalidCategory(lhsValue), let .invalidCategory(rhsValue)):
-            return lhsValue == rhsValue
-
-        case (let .invalidRepositoryType(lhsValue), let .invalidRepositoryType(rhsValue)):
-            return lhsValue == rhsValue
-
-        case (.missingAuthorization, .missingAuthorization),
-             (.missingOwner, .missingOwner):
-
-                return true
-
-        default:
-            return false
-        }
-    }
-
-}
-
-
 public struct ScanOptions: OptionsProtocol {