wip

Author: finestructure

Sources/App/Commands/Ingest.swift

@@ -16,6 +16,17 @@ import Vapor
 import Fluent
 
 
+enum Ingestion {
+    enum Error: Swift.Error {
+        case fetchMetadataFailed(owner: String, name: String, error: Swift.Error)
+        case findOrCreateRepositoryFailed(url: String, error: Swift.Error)
+        case invalidURL(String)
+        case noRepositoryMetadata(owner: String?, name: String?)
+        case repositorySaveFailed(owner: String?, name: String?, error: Swift.Error)
+    }
+}
+
+
 struct IngestCommand: AsyncCommand {
     typealias Signature = SPICommand.Signature
 
@@ -152,17 +163,82 @@ func ingestOriginal(client: Client, database: Database, package: Joined<Package,
 }
 
 
-func fetchMetadata(client: Client, package: Joined<Package, Repository>) async throws -> (Github.Metadata, Github.License?, Github.Readme?) {
+extension Ingestion {
+    static func ingestNew(client: Client, database: Database, package: Joined<Package, Repository>) async {
+        let result = await Result { () async throws(Ingestion.Error) -> Joined<Package, Repository> in
+            Current.logger().info("Ingesting \(package.package.url)")
+            let (metadata, license, readme) = try await fetchMetadata(client: client, package: package)
+            let repo = try await Result {
+                try await Repository.findOrCreate(on: database, for: package.model)
+            }.mapError {
+                Ingestion.Error.findOrCreateRepositoryFailed(url: package.package.url, error: $0)
+            }.get()
+
+            let s3Readme: S3Readme?
+            do throws(S3ReadmeError) {
+                s3Readme = try await storeS3Readme(client: client, repository: repo, metadata: metadata, readme: readme)
+            } catch {
+                // We don't want to fail ingestion in case storing the readme fails - warn and continue.
+                Current.logger().warning("storeS3Readme failed: \(error)")
+                s3Readme = .error("\(error)")
+            }
+
+            try await updateRepository(on: database, for: repo, metadata: metadata, licenseInfo: license, readmeInfo: readme, s3Readme: s3Readme)
+            return package
+        }
+
+        switch result {
+            case .success:
+                AppMetrics.ingestMetadataSuccessCount?.inc()
+            case .failure:
+                AppMetrics.ingestMetadataFailureCount?.inc()
+        }
+
+        do {
+            try await updatePackage(client: client, database: database, result: result, stage: .ingestion)
+        } catch {
+            Current.logger().report(error: error)
+        }
+    }
+
+
+    static func storeS3Readme(client: Client, repository: Repository, metadata: Github.Metadata, readme: Github.Readme?) async throws(S3ReadmeError) -> S3Readme? {
+        if let upstreamEtag = readme?.etag,
+           repository.s3Readme?.needsUpdate(upstreamEtag: upstreamEtag) ?? true,
+           let owner = metadata.repositoryOwner,
+           let repository = metadata.repositoryName,
+           let html = readme?.html {
+            let objectUrl = try await Current.storeS3Readme(owner, repository, html)
+            if let imagesToCache = readme?.imagesToCache, imagesToCache.isEmpty == false {
+                try await Current.storeS3ReadmeImages(client, imagesToCache)
+            }
+            return .cached(s3ObjectUrl: objectUrl, githubEtag: upstreamEtag)
+        } else {
+            return repository.s3Readme
+        }
+    }
+}
+
+func fetchMetadata(client: Client, package: Joined<Package, Repository>) async throws(Ingestion.Error) -> (Github.Metadata, Github.License?, Github.Readme?) {
     // Even though we get through a `Joined<Package, Repository>` as a parameter, it's
     // we must not rely on `repository` as it will be nil when a package is first ingested.
     // The only way to get `owner` and `repository` here is by parsing them from the URL.
-    let (owner, repository) = try Github.parseOwnerName(url: package.model.url)
+    let (owner, repository) = try Result {
+        try Github.parseOwnerName(url: package.model.url)
+    }.mapError { _ in
+        Ingestion.Error.invalidURL(package.model.url)
+    }.get()
 
-    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)
+    // First one should be an `async let` as well but it doesn't compile right now. Reported as
+    // https://github.com/swiftlang/swift/issues/76169
+    return (try await Result { try await Current.fetchMetadata(client, owner, repository) }
+        .mapError { Ingestion.Error.fetchMetadataFailed(owner: owner, name: repository, error: $0) }
+        .get(),
+            await license,
+            await readme)
 }
 
 
@@ -177,13 +253,9 @@ func updateRepository(on database: Database,
                       metadata: Github.Metadata,
                       licenseInfo: Github.License?,
                       readmeInfo: Github.Readme?,
-                      s3Readme: S3Readme?) async throws {
+                      s3Readme: S3Readme?) async throws(Ingestion.Error) {
     guard let repoMetadata = metadata.repository else {
-        if repository.$package.value == nil {
-            try await repository.$package.load(on: database)
-        }
-        throw AppError.genericError(repository.package.id,
-                                    "repository metadata is nil for package \(repository.name ?? "unknown")")
+        throw .noRepositoryMetadata(owner: repository.owner, name: repository.name)
     }
 
     repository.defaultBranch = repoMetadata.defaultBranch
@@ -209,7 +281,11 @@ func updateRepository(on database: Database,
     repository.stars = repoMetadata.stargazerCount
     repository.summary = repoMetadata.description
 
-    try await repository.save(on: database)
+    try await Result {
+        try await repository.save(on: database)
+    }.mapError {
+        Ingestion.Error.repositorySaveFailed(owner: repository.owner, name: repository.name, error: $0)
+    }.get()
 }
 
 

Sources/App/Core/Github.swift

@@ -19,24 +19,13 @@ import S3Store
 
 enum Github {
 
-    enum Error: LocalizedError {
+    enum Error: Swift.Error {
+        case decodeContentFailed(URI, Swift.Error)
         case missingToken
         case noBody
-        case invalidURI(Package.Id?, _ url: String)
+        case invalidURL(String)
+        case postRequestFailed(URI, Swift.Error)
         case requestFailed(HTTPStatus)
-
-        var errorDescription: String? {
-            switch self {
-                case .missingToken:
-                    return "missing Github API token"
-                case .noBody:
-                    return "no body"
-                case let .invalidURI(id, url):
-                    return "invalid URL: \(url) (id: \(id?.uuidString ?? "nil"))"
-                case .requestFailed(let statusCode):
-                    return "request failed with status code: \(statusCode)"
-            }
-        }
     }
 
     static var decoder: JSONDecoder {
@@ -60,13 +49,13 @@ enum Github {
         return response.status == .forbidden && limit == 0
     }
 
-    static func parseOwnerName(url: String) throws -> (owner: String, name: String) {
+    static func parseOwnerName(url: String) throws(Github.Error) -> (owner: String, name: String) {
         let parts = url
             .droppingGithubComPrefix
             .droppingGitExtension
             .split(separator: "/")
             .map(String.init)
-        guard parts.count == 2 else { throw Error.invalidURI(nil, url) }
+        guard parts.count == 2 else { throw Error.invalidURL(url) }
         return (owner: parts[0], name: parts[1])
     }
 
@@ -181,13 +170,18 @@ extension Github {
         var query: String
     }
 
-    static func fetchResource<T: Decodable>(_ type: T.Type, client: Client, query: GraphQLQuery) async throws -> T {
+    static func fetchResource<T: Decodable>(_ type: T.Type, client: Client, query: GraphQLQuery) async throws(Github.Error) -> T {
         guard let token = Current.githubToken() else {
             throw Error.missingToken
         }
 
-        let response = try await client.post(Self.graphQLApiUri, headers: defaultHeaders(with: token)) {
-            try $0.content.encode(query)
+        let response: ClientResponse
+        do {
+            response = try await client.post(Self.graphQLApiUri, headers: defaultHeaders(with: token)) {
+                try $0.content.encode(query)
+            }
+        } catch {
+            throw .postRequestFailed(Self.graphQLApiUri, error)
         }
 
         guard !isRateLimited(response) else {
@@ -200,10 +194,14 @@ extension Github {
             throw Error.requestFailed(response.status)
         }
 
-        return try response.content.decode(T.self, using: decoder)
+        do {
+            return try response.content.decode(T.self, using: decoder)
+        } catch {
+            throw .decodeContentFailed(Self.graphQLApiUri, error)
+        }
     }
 
-    static func fetchMetadata(client: Client, owner: String, repository: String) async throws -> Metadata {
+    static func fetchMetadata(client: Client, owner: String, repository: String) async throws(Github.Error) -> Metadata {
         struct Response<T: Decodable & Equatable>: Decodable, Equatable {
             var data: T
         }

Tests/AppTests/ErrorReportingTests.swift

@@ -34,15 +34,15 @@ class ErrorReportingTests: AppTestCase {
     func test_Ingestor_error_reporting() async throws {
         // setup
         try await Package(url: "1", processingStage: .reconciliation).save(on: app.db)
-        Current.fetchMetadata = { _, _, _ in throw Github.Error.invalidURI(nil, "1") }
+        Current.fetchMetadata = { _, _, _ in throw Github.Error.invalidURL("1") }
 
         // MUT
         try await ingest(client: app.client, database: app.db, mode: .limit(10))
 
         // validation
         logger.logs.withValue {
             XCTAssertEqual($0, [.init(level: .warning,
-                                      message: #"App.Github.Error.invalidURI(nil, "1")"#)])
+                                      message: #"App.Ingestion.Error.invalidURL("1")"#)])
         }
     }
 

Tests/AppTests/GithubTests.swift

@@ -33,11 +33,13 @@ class GithubTests: AppTestCase {
             XCTAssertEqual(res.owner, "foo")
             XCTAssertEqual(res.name, "bar")
         }
-        XCTAssertThrowsError(
-            try Github.parseOwnerName(url: "https://github.com/foo/bar/baz")
-        ) { error in
-            XCTAssertEqual(error.localizedDescription,
-                           "invalid URL: https://github.com/foo/bar/baz (id: nil)")
+        do {
+            _ = try Github.parseOwnerName(url: "https://github.com/foo/bar/baz")
+            XCTFail("Expected error")
+        } catch let Github.Error.invalidURL(url) {
+            XCTAssertEqual(url, "https://github.com/foo/bar/baz")
+        } catch {
+            XCTFail("Unexpected error: \(error)")
         }
     }
 
@@ -201,7 +203,7 @@ class GithubTests: AppTestCase {
             _ = try await Github.fetchMetadata(client: client, packageUrl: pkg.url)
             XCTFail("expected error to be thrown")
         } catch {
-            guard case Github.Error.invalidURI = error else {
+            guard case Github.Error.invalidURL = error else {
                 XCTFail("unexpected error: \(error.localizedDescription)")
                 return
             }
@@ -221,12 +223,15 @@ class GithubTests: AppTestCase {
         do {
             _ = try await Github.fetchMetadata(client: client, packageUrl: pkg.url)
             XCTFail("expected error to be thrown")
-        } catch {
+        } catch let Github.Error.decodeContentFailed(uri, error) {
             // validation
+            XCTAssertEqual(uri, "https://api.github.com/graphql")
             guard case DecodingError.dataCorrupted = error else {
                 XCTFail("unexpected error: \(error.localizedDescription)")
                 return
             }
+        } catch {
+            XCTFail("Unexpected error: \(error)")
         }
     }