[Collections] Improve error handling (#3471)

Make sure errors returned by `JSONPackageCollectionProvider` have good string representation.

rdar://77530631

Author: yim-lee

Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift

@@ -67,7 +67,7 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
         }
 
         if let errors = source.validate()?.errors() {
-            return callback(.failure(MultipleErrors(errors)))
+            return callback(.failure(JSONPackageCollectionProviderError.invalidSource("\(errors)")))
         }
 
         // Source is a local file
@@ -89,18 +89,18 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
             switch result {
             case .failure(HTTPClientError.badResponseStatusCode(let statusCode)):
                 if statusCode == 404 {
-                    return callback(.failure(Errors.collectionNotFound(source.url)))
+                    return callback(.failure(JSONPackageCollectionProviderError.collectionNotFound(source.url)))
                 } else {
-                    return callback(.failure(Errors.collectionUnavailable(source.url, statusCode)))
+                    return callback(.failure(JSONPackageCollectionProviderError.collectionUnavailable(source.url, statusCode)))
                 }
             case .failure(let error):
                 return callback(.failure(error))
             case .success(let response):
                 guard let contentLength = response.headers.get("Content-Length").first.flatMap(Int64.init) else {
-                    return callback(.failure(Errors.invalidResponse(source.url, "Missing Content-Length header")))
+                    return callback(.failure(JSONPackageCollectionProviderError.invalidResponse(source.url, "Missing Content-Length header")))
                 }
                 guard contentLength <= self.configuration.maximumSizeInBytes else {
-                    return callback(.failure(Errors.responseTooLarge(source.url, contentLength)))
+                    return callback(.failure(JSONPackageCollectionProviderError.responseTooLarge(source.url, contentLength)))
                 }
                 // next do a get request to get the actual content
                 var getOptions = self.makeRequestOptions(validResponseCodes: [200])
@@ -109,9 +109,9 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
                     switch result {
                     case .failure(HTTPClientError.badResponseStatusCode(let statusCode)):
                         if statusCode == 404 {
-                            return callback(.failure(Errors.collectionNotFound(source.url)))
+                            return callback(.failure(JSONPackageCollectionProviderError.collectionNotFound(source.url)))
                         } else {
-                            return callback(.failure(Errors.collectionUnavailable(source.url, statusCode)))
+                            return callback(.failure(JSONPackageCollectionProviderError.collectionUnavailable(source.url, statusCode)))
                         }
                     case .failure(let error):
                         return callback(.failure(error))
@@ -120,13 +120,13 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
                         // if not returning head and exceeding size
                         // TODO: store bad actors to prevent server DoS
                         guard let contentLength = response.headers.get("Content-Length").first.flatMap(Int64.init) else {
-                            return callback(.failure(Errors.invalidResponse(source.url, "Missing Content-Length header")))
+                            return callback(.failure(JSONPackageCollectionProviderError.invalidResponse(source.url, "Missing Content-Length header")))
                         }
                         guard contentLength < self.configuration.maximumSizeInBytes else {
-                            return callback(.failure(Errors.responseTooLarge(source.url, contentLength)))
+                            return callback(.failure(JSONPackageCollectionProviderError.responseTooLarge(source.url, contentLength)))
                         }
                         guard let body = response.body else {
-                            return callback(.failure(Errors.invalidResponse(source.url, "Body is empty")))
+                            return callback(.failure(JSONPackageCollectionProviderError.invalidResponse(source.url, "Body is empty")))
                         }
 
                         let certPolicyKey = self.sourceCertPolicy.certificatePolicyKey(for: source) ?? .default
@@ -175,15 +175,15 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
             }
             // Collection is unsigned
             guard let collection = try? self.decoder.decode(JSONModel.Collection.self, from: data) else {
-                return callback(.failure(Errors.invalidJSON(source.url)))
+                return callback(.failure(JSONPackageCollectionProviderError.invalidJSON(source.url)))
             }
             callback(self.makeCollection(from: collection, source: source, signature: nil))
         }
     }
 
     private func makeCollection(from collection: JSONModel.Collection, source: Model.CollectionSource, signature: Model.SignatureData?) -> Result<Model.Collection, Error> {
         if let errors = self.validator.validate(collection: collection)?.errors() {
-            return .failure(MultipleErrors(errors))
+            return .failure(JSONPackageCollectionProviderError.invalidCollection("\(errors)"))
         }
 
         var serializationOkay = true
@@ -345,27 +345,31 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
             )
         }
     }
+}
 
-    public enum Errors: Error, Equatable, CustomStringConvertible {
-        case invalidJSON(URL)
-        case invalidResponse(URL, String)
-        case responseTooLarge(URL, Int64)
-        case collectionNotFound(URL)
-        case collectionUnavailable(URL, Int)
-
-        public var description: String {
-            switch self {
-            case .invalidJSON(let url):
-                return "The package collection at \(url.absoluteString) contains invalid JSON."
-            case .invalidResponse(let url, let message):
-                return "Received invalid response for package collection at \(url.absoluteString): \(message)"
-            case .responseTooLarge(let url, _):
-                return "The package collection at \(url.absoluteString) is too large."
-            case .collectionNotFound(let url):
-                return "No package collection found at \(url.absoluteString). Please make sure the URL is correct."
-            case .collectionUnavailable(let url, _):
-                return "The package collection at \(url.absoluteString) is unavailable. Please make sure the URL is correct or try again later."
-            }
+public enum JSONPackageCollectionProviderError: Error, Equatable, CustomStringConvertible {
+    case invalidSource(String)
+    case invalidJSON(URL)
+    case invalidCollection(String)
+    case invalidResponse(URL, String)
+    case responseTooLarge(URL, Int64)
+    case collectionNotFound(URL)
+    case collectionUnavailable(URL, Int)
+
+    public var description: String {
+        switch self {
+        case .invalidSource(let errorMessage), .invalidCollection(let errorMessage):
+            return errorMessage
+        case .invalidJSON(let url):
+            return "The package collection at \(url.absoluteString) contains invalid JSON."
+        case .invalidResponse(let url, let message):
+            return "Received invalid response for package collection at \(url.absoluteString): \(message)"
+        case .responseTooLarge(let url, _):
+            return "The package collection at \(url.absoluteString) is too large."
+        case .collectionNotFound(let url):
+            return "No package collection found at \(url.absoluteString). Please make sure the URL is correct."
+        case .collectionUnavailable(let url, _):
+            return "The package collection at \(url.absoluteString) is unavailable. Please make sure the URL is correct or try again later."
         }
     }
 }

Tests/PackageCollectionsTests/JSONPackageCollectionProviderTests.swift

@@ -124,13 +124,10 @@ class JSONPackageCollectionProviderTests: XCTestCase {
         httpClient.configuration.retryStrategy = .none
         let provider = JSONPackageCollectionProvider(httpClient: httpClient, diagnosticsEngine: DiagnosticsEngine())
         XCTAssertThrowsError(try tsc_await { callback in provider.get(source, callback: callback) }, "expected error", { error in
-            guard let internalError = (error as? MultipleErrors)?.errors.first else {
+            guard case .invalidSource(let errorMessage) = error as? JSONPackageCollectionProviderError else {
                 return XCTFail("invalid error \(error)")
             }
-            guard let validationError = internalError as? ValidationError, case .other(let message) = validationError else {
-                return XCTFail("invalid error \(error)")
-            }
-            XCTAssertTrue(message.contains("Scheme (\"ftp\") not allowed: \(url.absoluteString)"))
+            XCTAssertTrue(errorMessage.contains("Scheme (\"ftp\") not allowed: \(url.absoluteString)"))
         })
     }
 
@@ -152,7 +149,7 @@ class JSONPackageCollectionProviderTests: XCTestCase {
         let configuration = JSONPackageCollectionProvider.Configuration(maximumSizeInBytes: 10)
         let provider = JSONPackageCollectionProvider(configuration: configuration, httpClient: httpClient, diagnosticsEngine: DiagnosticsEngine())
         XCTAssertThrowsError(try tsc_await { callback in provider.get(source, callback: callback) }, "expected error", { error in
-            XCTAssertEqual(error as? JSONPackageCollectionProvider.Errors, .responseTooLarge(url, maxSize * 2))
+            XCTAssertEqual(error as? JSONPackageCollectionProviderError, .responseTooLarge(url, maxSize * 2))
         })
     }
 
@@ -181,7 +178,7 @@ class JSONPackageCollectionProviderTests: XCTestCase {
         let configuration = JSONPackageCollectionProvider.Configuration(maximumSizeInBytes: 10)
         let provider = JSONPackageCollectionProvider(configuration: configuration, httpClient: httpClient, diagnosticsEngine: DiagnosticsEngine())
         XCTAssertThrowsError(try tsc_await { callback in provider.get(source, callback: callback) }, "expected error", { error in
-            XCTAssertEqual(error as? JSONPackageCollectionProvider.Errors, .responseTooLarge(url, maxSize * 2))
+            XCTAssertEqual(error as? JSONPackageCollectionProviderError, .responseTooLarge(url, maxSize * 2))
         })
     }
 
@@ -201,7 +198,7 @@ class JSONPackageCollectionProviderTests: XCTestCase {
         let configuration = JSONPackageCollectionProvider.Configuration(maximumSizeInBytes: 10)
         let provider = JSONPackageCollectionProvider(configuration: configuration, httpClient: httpClient, diagnosticsEngine: DiagnosticsEngine())
         XCTAssertThrowsError(try tsc_await { callback in provider.get(source, callback: callback) }, "expected error", { error in
-            XCTAssertEqual(error as? JSONPackageCollectionProvider.Errors, .invalidResponse(url, "Missing Content-Length header"))
+            XCTAssertEqual(error as? JSONPackageCollectionProviderError, .invalidResponse(url, "Missing Content-Length header"))
         })
     }
 
@@ -249,7 +246,7 @@ class JSONPackageCollectionProviderTests: XCTestCase {
         httpClient.configuration.retryStrategy = .none
         let provider = JSONPackageCollectionProvider(httpClient: httpClient, diagnosticsEngine: DiagnosticsEngine())
         XCTAssertThrowsError(try tsc_await { callback in provider.get(source, callback: callback) }, "expected error", { error in
-            XCTAssertEqual(error as? JSONPackageCollectionProvider.Errors, .collectionUnavailable(url, statusCode))
+            XCTAssertEqual(error as? JSONPackageCollectionProviderError, .collectionUnavailable(url, statusCode))
         })
     }
 
@@ -275,7 +272,7 @@ class JSONPackageCollectionProviderTests: XCTestCase {
         httpClient.configuration.retryStrategy = .none
         let provider = JSONPackageCollectionProvider(httpClient: httpClient, diagnosticsEngine: DiagnosticsEngine())
         XCTAssertThrowsError(try tsc_await { callback in provider.get(source, callback: callback) }, "expected error", { error in
-            XCTAssertEqual(error as? JSONPackageCollectionProvider.Errors, .collectionUnavailable(url, statusCode))
+            XCTAssertEqual(error as? JSONPackageCollectionProviderError, .collectionUnavailable(url, statusCode))
         })
     }
 
@@ -294,7 +291,7 @@ class JSONPackageCollectionProviderTests: XCTestCase {
         httpClient.configuration.retryStrategy = .none
         let provider = JSONPackageCollectionProvider(httpClient: httpClient, diagnosticsEngine: DiagnosticsEngine())
         XCTAssertThrowsError(try tsc_await { callback in provider.get(source, callback: callback) }, "expected error", { error in
-            XCTAssertEqual(error as? JSONPackageCollectionProvider.Errors, .collectionNotFound(url))
+            XCTAssertEqual(error as? JSONPackageCollectionProviderError, .collectionNotFound(url))
         })
     }
 
@@ -319,7 +316,7 @@ class JSONPackageCollectionProviderTests: XCTestCase {
         httpClient.configuration.retryStrategy = .none
         let provider = JSONPackageCollectionProvider(httpClient: httpClient, diagnosticsEngine: DiagnosticsEngine())
         XCTAssertThrowsError(try tsc_await { callback in provider.get(source, callback: callback) }, "expected error", { error in
-            XCTAssertEqual(error as? JSONPackageCollectionProvider.Errors, .collectionNotFound(url))
+            XCTAssertEqual(error as? JSONPackageCollectionProviderError, .collectionNotFound(url))
         })
     }
 
@@ -347,7 +344,7 @@ class JSONPackageCollectionProviderTests: XCTestCase {
         let provider = JSONPackageCollectionProvider(httpClient: httpClient, diagnosticsEngine: DiagnosticsEngine())
         let source = PackageCollectionsModel.CollectionSource(type: .json, url: url)
         XCTAssertThrowsError(try tsc_await { callback in provider.get(source, callback: callback) }, "expected error", { error in
-            XCTAssertEqual(error as? JSONPackageCollectionProvider.Errors, .invalidJSON(url))
+            XCTAssertEqual(error as? JSONPackageCollectionProviderError, .invalidJSON(url))
         })
     }