Turn Ingestion.fetchMetadata into throws(Github.Error)

SwiftPackageIndex · Dec 14, 2024 · ecae5b9 · ecae5b9
1 parent 36f0bf4
commit ecae5b9
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 15 deletions.
diff --git a/Sources/App/Commands/Ingest.swift b/Sources/App/Commands/Ingest.swift
@@ -250,12 +250,25 @@ extension Ingestion {
     }
 
 
-    static func fetchMetadata(client: Client, package: Package, owner: String, repository: String) async throws -> (Github.Metadata, Github.License?, Github.Readme?) {
+    static func fetchMetadata(client: Client, package: Package, owner: String, repository: String) async throws(Github.Error) -> (Github.Metadata, Github.License?, Github.Readme?) {
         async let metadata = try await Current.fetchMetadata(client, owner, repository)
         async let license = await Current.fetchLicense(client, owner, repository)
         async let readme = await Current.fetchReadme(client, owner, repository)
 
-        return try await (metadata, license, readme)
+        do {
+            return try await (metadata, license, readme)
+        } catch let error as Github.Error {
+            throw error
+        } catch {
+            // This whole do { ... } catch { ... } should be unnecessary - it's a workaround for
+            // https://github.com/swiftlang/swift/issues/76169
+            assert(false, "Unexpected error type: \(type(of: error))")
+            // We need to throw _something_ here (we should never hit this codepath though)
+            throw Github.Error.requestFailed(.internalServerError)
+            // We could theoretically avoid this whole second catch and just do
+            //   error as! GithubError
+            // but let's play it safe and not risk a server crash, unlikely as it may be.
+        }
     }
 }
 

diff --git a/Sources/App/Core/AppEnvironment.swift b/Sources/App/Core/AppEnvironment.swift
@@ -25,7 +25,7 @@ import FoundationNetworking
 struct AppEnvironment: Sendable {
     var fetchHTTPStatusCode: @Sendable (_ url: String) async throws -> HTTPStatus
     var fetchLicense: @Sendable (_ client: Client, _ owner: String, _ repository: String) async -> Github.License?
-    var fetchMetadata: @Sendable (_ client: Client, _ owner: String, _ repository: String) async throws -> Github.Metadata
+    var fetchMetadata: @Sendable (_ client: Client, _ owner: String, _ repository: String) async throws(Github.Error) -> Github.Metadata
     var fetchReadme: @Sendable (_ client: Client, _ owner: String, _ repository: String) async -> Github.Readme?
     var fetchS3Readme: @Sendable (_ client: Client, _ owner: String, _ repository: String) async throws -> String
     var fileManager: FileManager
@@ -86,7 +86,7 @@ extension AppEnvironment {
     static let live = AppEnvironment(
         fetchHTTPStatusCode: { url in try await Networking.fetchHTTPStatusCode(url) },
         fetchLicense: { client, owner, repo in await Github.fetchLicense(client:client, owner: owner, repository: repo) },
-        fetchMetadata: { client, owner, repo in try await Github.fetchMetadata(client:client, owner: owner, repository: repo) },
+        fetchMetadata: { client, owner, repo throws(Github.Error) in try await Github.fetchMetadata(client:client, owner: owner, repository: repo) },
         fetchReadme: { client, owner, repo in await Github.fetchReadme(client:client, owner: owner, repository: repo) },
         fetchS3Readme: { client, owner, repo in try await S3Readme.fetchReadme(client:client, owner: owner, repository: repo) },
         fileManager: .live,

diff --git a/Tests/AppTests/ErrorReportingTests.swift b/Tests/AppTests/ErrorReportingTests.swift
@@ -34,7 +34,7 @@ class ErrorReportingTests: AppTestCase {
     func test_Ingestor_error_reporting() async throws {
         // setup
         try await Package(id: .id0, url: "1", processingStage: .reconciliation).save(on: app.db)
-        Current.fetchMetadata = { _, _, _ in throw Github.Error.invalidURL("1") }
+        Current.fetchMetadata = { _, _, _ throws(Github.Error) in throw Github.Error.invalidURL("1") }
 
         try await withDependencies {
             $0.date.now = .now

diff --git a/Tests/AppTests/IngestorTests.swift b/Tests/AppTests/IngestorTests.swift
@@ -65,16 +65,12 @@ class IngestorTests: AppTestCase {
     func test_ingest_continue_on_error() async throws {
         // Test completion of ingestion despite early error
         // setup
-        enum TestError: Error, Equatable {
-            case badRequest
-        }
-
         let packages = try await savePackages(on: app.db, ["https://github.com/foo/1",
                                                            "https://github.com/foo/2"], processingStage: .reconciliation)
             .map(Joined<Package, Repository>.init(model:))
-        Current.fetchMetadata = { _, owner, repository in
+        Current.fetchMetadata = { _, owner, repository throws(Github.Error) in
             if owner == "foo" && repository == "1" {
-                throw TestError.badRequest
+                throw Github.Error.requestFailed(.badRequest)
             }
             return .mock(owner: owner, repository: repository)
         }
@@ -328,11 +324,10 @@ class IngestorTests: AppTestCase {
         let urls = ["https://github.com/foo/1",
                     "https://github.com/foo/2",
                     "https://github.com/foo/3"]
-        let packages = try await savePackages(on: app.db, urls.asURLs,
-                                              processingStage: .reconciliation)
-        Current.fetchMetadata = { _, owner, repository in
+        try await savePackages(on: app.db, urls.asURLs, processingStage: .reconciliation)
+        Current.fetchMetadata = { _, owner, repository throws(Github.Error) in
             if owner == "foo" && repository == "2" {
-                throw AppError.genericError(packages[1].id, "error 2")
+                throw Github.Error.requestFailed(.badRequest)
             }
             return .mock(owner: owner, repository: repository)
         }