Merge pull request #623 from rgoldberg/533-errors

Improve error messages

Sources/mas/AppStore/Downloader.swift

@@ -10,21 +10,53 @@ import CommerceKit
 import PromiseKit
 import StoreFoundation
 
-/// Downloads a list of apps, one after the other, printing progress to the console.
+/// Sequentially downloads apps, printing progress to the console.
+///
+/// Verifies that each supplied app ID is valid before attempting to download.
+///
+/// - Parameters:
+///   - unverifiedAppIDs: The app IDs of the apps to be verified and downloaded.
+///   - searcher: The `AppStoreSearcher` used to verify app IDs.
+///   - purchasing: Flag indicating if the apps will be purchased. Only works for free apps. Defaults to false.
+/// - Returns: A `Promise` that completes when the downloads are complete. If any fail,
+///   the promise is rejected with the first error, after all remaining downloads are attempted.
+func downloadApps(
+    withAppIDs unverifiedAppIDs: [AppID],
+    verifiedBy searcher: AppStoreSearcher,
+    purchasing: Bool = false
+) -> Promise<Void> {
+    when(resolved: unverifiedAppIDs.map { searcher.lookup(appID: $0) })
+        .then { results in
+            downloadApps(
+                withAppIDs:
+                    results.compactMap { result in
+                        switch result {
+                        case .fulfilled(let searchResult):
+                            return searchResult.trackId
+                        case .rejected(let error):
+                            printError(String(describing: error))
+                            return nil
+                        }
+                    },
+                purchasing: purchasing
+            )
+        }
+}
+
+/// Sequentially downloads apps, printing progress to the console.
 ///
 /// - Parameters:
-///   - appIDs: The IDs of the apps to be downloaded
-///   - purchase: Flag indicating whether the apps needs to be purchased.
-///     Only works for free apps. Defaults to false.
+///   - appIDs: The app IDs of the apps to be downloaded.
+///   - purchasing: Flag indicating if the apps will be purchased. Only works for free apps. Defaults to false.
 /// - Returns: A promise that completes when the downloads are complete. If any fail,
 ///   the promise is rejected with the first error, after all remaining downloads are attempted.
-func downloadAll(_ appIDs: [AppID], purchase: Bool = false) -> Promise<Void> {
+func downloadApps(withAppIDs appIDs: [AppID], purchasing: Bool = false) -> Promise<Void> {
     var firstError: Error?
     return
         appIDs
         .reduce(Guarantee.value(())) { previous, appID in
             previous.then {
-                downloadWithRetries(appID, purchase: purchase)
+                downloadApp(withAppID: appID, purchasing: purchasing)
                     .recover { error in
                         if firstError == nil {
                             firstError = error
@@ -39,10 +71,15 @@ func downloadAll(_ appIDs: [AppID], purchase: Bool = false) -> Promise<Void> {
         }
 }
 
-private func downloadWithRetries(_ appID: AppID, purchase: Bool = false, attempts: Int = 3) -> Promise<Void> {
-    SSPurchase().perform(appID: appID, purchase: purchase)
+private func downloadApp(
+    withAppID appID: AppID,
+    purchasing: Bool = false,
+    withAttemptCount attemptCount: UInt32 = 3
+) -> Promise<Void> {
+    SSPurchase()
+        .perform(appID: appID, purchasing: purchasing)
         .recover { error in
-            guard attempts > 1 else {
+            guard attemptCount > 1 else {
                 throw error
             }
 
@@ -54,9 +91,9 @@ private func downloadWithRetries(_ appID: AppID, purchase: Bool = false, attempt
                 throw error
             }
 
-            let attempts = attempts - 1
+            let attemptCount = attemptCount - 1
             printWarning((downloadError ?? error).localizedDescription)
-            printWarning("Trying again up to \(attempts) more \(attempts == 1 ? "time" : "times").")
-            return downloadWithRetries(appID, purchase: purchase, attempts: attempts)
+            printWarning("Trying again up to \(attemptCount) more \(attemptCount == 1 ? "time" : "times").")
+            return downloadApp(withAppID: appID, purchasing: purchasing, withAttemptCount: attemptCount)
         }
 }

Sources/mas/AppStore/SSPurchase.swift

@@ -11,7 +11,7 @@ import PromiseKit
 import StoreFoundation
 
 extension SSPurchase {
-    func perform(appID: AppID, purchase: Bool) -> Promise<Void> {
+    func perform(appID: AppID, purchasing: Bool) -> Promise<Void> {
         var parameters: [String: Any] = [
             "productType": "C",
             "price": 0,
@@ -20,9 +20,11 @@ extension SSPurchase {
             "appExtVrsId": 0,
         ]
 
-        if purchase {
+        if purchasing {
             parameters["macappinstalledconfirmed"] = 1
             parameters["pricingParameters"] = "STDQ"
+            // Possibly unnecessary…
+            isRedownload = false
         } else {
             parameters["pricingParameters"] = "STDRDL"
         }
@@ -35,11 +37,6 @@ extension SSPurchase {
 
         itemIdentifier = appID
 
-        // Not sure if this is needed…
-        if purchase {
-            isRedownload = false
-        }
-
         downloadMetadata = SSDownloadMetadata()
         downloadMetadata.kind = "software"
         downloadMetadata.itemIdentifier = appID

Sources/mas/Commands/Home.swift

@@ -26,9 +26,7 @@ extension MAS {
         }
 
         func run(searcher: AppStoreSearcher) throws {
-            guard let result = try searcher.lookup(appID: appID).wait() else {
-                throw MASError.noSearchResultsFound
-            }
+            let result = try searcher.lookup(appID: appID).wait()
 
             guard let url = URL(string: result.trackViewUrl) else {
                 throw MASError.runtimeError("Unable to construct URL from: \(result.trackViewUrl)")

Sources/mas/Commands/Info.swift

@@ -27,11 +27,7 @@ extension MAS {
 
         func run(searcher: AppStoreSearcher) throws {
             do {
-                guard let result = try searcher.lookup(appID: appID).wait() else {
-                    throw MASError.noSearchResultsFound
-                }
-
-                print(AppInfoFormatter.format(app: result))
+                print(AppInfoFormatter.format(app: try searcher.lookup(appID: appID).wait()))
             } catch {
                 throw error as? MASError ?? .searchFailed
             }

Sources/mas/Commands/Install.swift

@@ -18,15 +18,15 @@ extension MAS {
 
         @Flag(help: "Force reinstall")
         var force = false
-        @Argument(help: "App ID(s)")
+        @Argument(help: ArgumentHelp("App ID", valueName: "app-id"))
         var appIDs: [AppID]
 
         /// Runs the command.
         func run() throws {
-            try run(appLibrary: SoftwareMapAppLibrary())
+            try run(appLibrary: SoftwareMapAppLibrary(), searcher: ITunesSearchAppStoreSearcher())
         }
 
-        func run(appLibrary: AppLibrary) throws {
+        func run(appLibrary: AppLibrary, searcher: AppStoreSearcher) throws {
             // Try to download applications with given identifiers and collect results
             let appIDs = appIDs.filter { appID in
                 if let appName = appLibrary.installedApps(withAppID: appID).first?.appName, !force {
@@ -38,7 +38,7 @@ extension MAS {
             }
 
             do {
-                try downloadAll(appIDs).wait()
+                try downloadApps(withAppIDs: appIDs, verifiedBy: searcher).wait()
             } catch {
                 throw error as? MASError ?? .downloadFailed(error: error as NSError)
             }

Sources/mas/Commands/Lucky.swift

@@ -38,7 +38,6 @@ extension MAS {
             do {
                 let results = try searcher.search(for: searchTerm).wait()
                 guard let result = results.first else {
-                    printError("No results found")
                     throw MASError.noSearchResultsFound
                 }
 
@@ -66,7 +65,7 @@ extension MAS {
                 printWarning("\(appName) is already installed")
             } else {
                 do {
-                    try downloadAll([appID]).wait()
+                    try downloadApps(withAppIDs: [appID]).wait()
                 } catch {
                     throw error as? MASError ?? .downloadFailed(error: error as NSError)
                 }

Sources/mas/Commands/Open.swift

@@ -63,9 +63,7 @@ private func openMacAppStore() -> Promise<Void> {
 }
 
 private func openInMacAppStore(pageForAppID appID: AppID, searcher: AppStoreSearcher) throws {
-    guard let result = try searcher.lookup(appID: appID).wait() else {
-        throw MASError.runtimeError("Unknown app ID \(appID)")
-    }
+    let result = try searcher.lookup(appID: appID).wait()
 
     guard var urlComponents = URLComponents(string: result.trackViewUrl) else {
         throw MASError.runtimeError("Unable to construct URL from: \(result.trackViewUrl)")

Sources/mas/Commands/Outdated.swift

@@ -30,11 +30,22 @@ extension MAS {
             _ = try when(
                 fulfilled:
                     appLibrary.installedApps.map { installedApp in
-                        firstly {
-                            searcher.lookup(appID: installedApp.itemIdentifier.appIDValue)
-                        }
-                        .done { storeApp in
-                            guard let storeApp else {
+                        searcher.lookup(appID: installedApp.itemIdentifier.appIDValue)
+                            .done { storeApp in
+                                if installedApp.isOutdatedWhenComparedTo(storeApp) {
+                                    print(
+                                        """
+                                        \(installedApp.itemIdentifier) \(installedApp.appName) \
+                                        (\(installedApp.bundleVersion) -> \(storeApp.version))
+                                        """
+                                    )
+                                }
+                            }
+                            .recover { error in
+                                guard case MASError.unknownAppID = error else {
+                                    throw error
+                                }
+
                                 if verbose {
                                     printWarning(
                                         """
@@ -43,18 +54,7 @@ extension MAS {
                                         """
                                     )
                                 }
-                                return
-                            }
-
-                            if installedApp.isOutdatedWhenComparedTo(storeApp) {
-                                print(
-                                    """
-                                    \(installedApp.itemIdentifier) \(installedApp.appName) \
-                                    (\(installedApp.bundleVersion) -> \(storeApp.version))
-                                    """
-                                )
                             }
-                        }
                     }
             )
             .wait()

Sources/mas/Commands/Purchase.swift

@@ -15,15 +15,15 @@ extension MAS {
             abstract: "\"Purchase\" and install free apps from the Mac App Store"
         )
 
-        @Argument(help: "App ID(s)")
+        @Argument(help: ArgumentHelp("App ID", valueName: "app-id"))
         var appIDs: [AppID]
 
         /// Runs the command.
         func run() throws {
-            try run(appLibrary: SoftwareMapAppLibrary())
+            try run(appLibrary: SoftwareMapAppLibrary(), searcher: ITunesSearchAppStoreSearcher())
         }
 
-        func run(appLibrary: AppLibrary) throws {
+        func run(appLibrary: AppLibrary, searcher: AppStoreSearcher) throws {
             // Try to download applications with given identifiers and collect results
             let appIDs = appIDs.filter { appID in
                 if let appName = appLibrary.installedApps(withAppID: appID).first?.appName {
@@ -35,7 +35,7 @@ extension MAS {
             }
 
             do {
-                try downloadAll(appIDs, purchase: true).wait()
+                try downloadApps(withAppIDs: appIDs, verifiedBy: searcher, purchasing: true).wait()
             } catch {
                 throw error as? MASError ?? .downloadFailed(error: error as NSError)
             }

Sources/mas/Commands/Uninstall.swift

@@ -32,12 +32,12 @@ extension MAS {
                 throw MASError.macOSUserMustBeRoot
             }
 
-            guard let username = getSudoUsername() else {
+            guard let username = ProcessInfo.processInfo.sudoUsername else {
                 throw MASError.runtimeError("Could not determine the original username")
             }
 
             guard
-                let uid = getSudoUID(),
+                let uid = ProcessInfo.processInfo.sudoUID,
                 seteuid(uid) == 0
             else {
                 throw MASError.runtimeError("Failed to switch effective user from 'root' to '\(username)'")

Sources/mas/Commands/Upgrade.swift

@@ -18,7 +18,7 @@ extension MAS {
                 "Upgrade outdated app(s) installed from the Mac App Store"
         )
 
-        @Argument(help: "App ID(s)/app name(s)")
+        @Argument(help: ArgumentHelp("App ID/app name", valueName: "app-id-or-name"))
         var appIDOrNames: [String] = []
 
         /// Runs the command.
@@ -35,7 +35,6 @@ extension MAS {
             }
 
             guard !apps.isEmpty else {
-                printWarning("Nothing found to upgrade")
                 return
             }
 
@@ -46,7 +45,7 @@ extension MAS {
             )
 
             do {
-                try downloadAll(apps.map(\.installedApp.itemIdentifier.appIDValue)).wait()
+                try downloadApps(withAppIDs: apps.map(\.installedApp.itemIdentifier.appIDValue)).wait()
             } catch {
                 throw error as? MASError ?? .downloadFailed(error: error as NSError)
             }
@@ -59,28 +58,39 @@ extension MAS {
             let apps =
                 appIDOrNames.isEmpty
                 ? appLibrary.installedApps
-                : appIDOrNames.flatMap { appID in
-                    if let appID = AppID(appID) {
+                : appIDOrNames.flatMap { appIDOrName in
+                    if let appID = AppID(appIDOrName) {
                         // argument is an AppID, lookup apps by id using argument
-                        return appLibrary.installedApps(withAppID: appID)
+                        let installedApps = appLibrary.installedApps(withAppID: appID)
+                        if installedApps.isEmpty {
+                            printError(appID.unknownMessage)
+                        }
+                        return installedApps
                     }
 
                     // argument is not an AppID, lookup apps by name using argument
-                    return appLibrary.installedApps(named: appID)
+                    let installedApps = appLibrary.installedApps(named: appIDOrName)
+                    if installedApps.isEmpty {
+                        printError("Unknown app name '\(appIDOrName)'")
+                    }
+                    return installedApps
                 }
 
             let promises = apps.map { installedApp in
                 // only upgrade apps whose local version differs from the store version
-                firstly {
-                    searcher.lookup(appID: installedApp.itemIdentifier.appIDValue)
-                }
-                .map { result -> (SoftwareProduct, SearchResult)? in
-                    guard let storeApp = result, installedApp.isOutdatedWhenComparedTo(storeApp) else {
-                        return nil
+                searcher.lookup(appID: installedApp.itemIdentifier.appIDValue)
+                    .map { storeApp -> (SoftwareProduct, SearchResult)? in
+                        guard installedApp.isOutdatedWhenComparedTo(storeApp) else {
+                            return nil
+                        }
+                        return (installedApp, storeApp)
+                    }
+                    .recover { error -> Promise<(SoftwareProduct, SearchResult)?> in
+                        guard case MASError.unknownAppID = error else {
+                            return Promise(error: error)
+                        }
+                        return .value(nil)
                     }
-
-                    return (installedApp, storeApp)
-                }
             }
 
             return try when(fulfilled: promises).wait().compactMap { $0 }

Sources/mas/Commands/Vendor.swift

@@ -26,9 +26,7 @@ extension MAS {
         }
 
         func run(searcher: AppStoreSearcher) throws {
-            guard let result = try searcher.lookup(appID: appID).wait() else {
-                throw MASError.noSearchResultsFound
-            }
+            let result = try searcher.lookup(appID: appID).wait()
 
             guard let urlString = result.sellerUrl else {
                 throw MASError.noVendorWebsite