Merge pull request #1589 from ebkr/local-import-cleanup-pt2

Further cleaning up LocalModInstaller

src/components/importing/LocalFileImportModal.vue

@@ -268,7 +268,7 @@ export default class LocalFileImportModal extends Vue {
         this.$emit("close-modal");
     }
 
-    private importFile() {
+    private async importFile() {
         if (this.fileToImport === null) {
             return;
         }
@@ -318,27 +318,25 @@ export default class LocalFileImportModal extends Vue {
         this.resultingManifest.setDescription(this.modDescription.trim());
         this.resultingManifest.setAuthorName(this.modAuthor.trim());
 
-        const installCallback = (async (success: boolean, error: any | null) => {
-            if (!success && error !== null) {
-                this.$store.commit("error/handleError", R2Error.fromThrownValue(error));
-                return;
-            }
-            const updatedModListResult = await ProfileModList.getModList(profile);
-            if (updatedModListResult instanceof R2Error) {
-                this.$store.commit("error/handleError", updatedModListResult);
-                return;
+        try {
+            if (this.fileToImport.endsWith(".zip")) {
+                await LocalModInstallerProvider.instance.extractToCacheWithManifestData(profile, this.fileToImport, this.resultingManifest);
+            } else {
+                await LocalModInstallerProvider.instance.placeFileInCache(profile, this.fileToImport, this.resultingManifest);
             }
-            await this.$store.dispatch("profile/updateModList", updatedModListResult);
-            this.emitClose();
-        });
+        } catch (e) {
+            this.$store.commit("error/handleError", R2Error.fromThrownValue(e));
+            return;
+        }
 
-        if (this.fileToImport.endsWith(".zip")) {
-            LocalModInstallerProvider.instance.extractToCacheWithManifestData(profile, this.fileToImport, this.resultingManifest, installCallback);
+        const updatedModListResult = await ProfileModList.getModList(profile);
+        if (updatedModListResult instanceof R2Error) {
+            this.$store.commit("error/handleError", updatedModListResult);
         } else {
-            LocalModInstallerProvider.instance.placeFileInCache(profile, this.fileToImport, this.resultingManifest, installCallback);
+            await this.$store.dispatch("profile/updateModList", updatedModListResult);
+            this.emitClose();
         }
     }
-
 }
 
 interface ImportFieldAttributes {

src/model/errors/R2Error.ts

@@ -27,3 +27,12 @@ export default class R2Error extends Error {
         }
     }
 }
+
+/**
+ * Throws the given value if it's an instance of R2Error.
+ */
+export function throwForR2Error(maybeError: unknown) {
+    if (maybeError instanceof R2Error) {
+        throw maybeError;
+    }
+}

src/providers/ror2/installing/LocalModInstallerProvider.ts

@@ -1,5 +1,4 @@
 import ProviderUtils from '../../generic/ProviderUtils';
-import R2Error from '../../../model/errors/R2Error';
 import { ImmutableProfile } from '../../../model/Profile';
 import ManifestV2 from '../../../model/ManifestV2';
 
@@ -25,7 +24,7 @@ export default abstract class LocalModInstallerProvider {
      * @param zipFile   Path to the zip file.
      * @param callback  Callback to report if the extraction was successful.
      */
-    public abstract extractToCacheWithManifestData(profile: ImmutableProfile, zipFile: string, manifest: ManifestV2, callback: (success: boolean, error: R2Error | null) => void): Promise<void>;
-    public abstract placeFileInCache(profile: ImmutableProfile, file: string, manifest: ManifestV2, callback: (success: boolean, error: R2Error | null) => void): Promise<void>;
+    public abstract extractToCacheWithManifestData(profile: ImmutableProfile, zipFile: string, manifest: ManifestV2): Promise<void>;
+    public abstract placeFileInCache(profile: ImmutableProfile, file: string, manifest: ManifestV2): Promise<void>;
 
 }

src/r2mm/installing/LocalModInstaller.ts

@@ -1,4 +1,4 @@
-import R2Error from '../../model/errors/R2Error';
+import R2Error, { throwForR2Error } from '../../model/errors/R2Error';
 import ManifestV2 from '../../model/ManifestV2';
 import ProfileInstallerProvider from '../../providers/ror2/installing/ProfileInstallerProvider';
 import ZipExtract from './ZipExtract';
@@ -42,58 +42,27 @@ export default class LocalModInstaller extends LocalModInstallerProvider {
         await FsProvider.instance.writeFile(manifestPath, JSON.stringify(manifest));
     }
 
-    public async extractToCacheWithManifestData(profile: ImmutableProfile, zipFile: string, manifest: ManifestV2, callback: (success: boolean, error: R2Error | null) => void) {
+    public async extractToCacheWithManifestData(profile: ImmutableProfile, zipFile: string, manifest: ManifestV2) {
         const cacheDirectory: string = await this.initialiseCacheDirectory(manifest);
-        await ZipExtract.extractOnly(
-            zipFile,
-            cacheDirectory,
-            async success => {
-                if (success) {
-                    try {
-                        await this.writeManifestToCache(cacheDirectory, manifest);
-                    } catch (e) {
-                        callback(false, R2Error.fromThrownValue(e));
-                        return Promise.resolve();
-                    }
-                    await ProfileInstallerProvider.instance.uninstallMod(manifest, profile);
-                    const profileInstallResult = await ProfileInstallerProvider.instance.installMod(manifest, profile);
-                    if (profileInstallResult instanceof R2Error) {
-                        callback(false, profileInstallResult);
-                        return Promise.resolve();
-                    }
-                    const modListInstallResult = await ProfileModList.addMod(manifest, profile);
-                    if (modListInstallResult instanceof R2Error) {
-                        callback(false, modListInstallResult);
-                        return Promise.resolve();
-                    }
-                    callback(true, null);
-                    return Promise.resolve();
-                }
-            }
-        );
+        await ZipExtract.extractOnly(zipFile, cacheDirectory);
+        await this.writeManifestToCache(cacheDirectory, manifest);
+        await ProfileInstallerProvider.instance.uninstallMod(manifest, profile);
+        throwForR2Error(await ProfileInstallerProvider.instance.installMod(manifest, profile));
+        throwForR2Error(await ProfileModList.addMod(manifest, profile));
     }
 
-    public async placeFileInCache(profile: ImmutableProfile, file: string, manifest: ManifestV2, callback: (success: boolean, error: (R2Error | null)) => void) {
+    public async placeFileInCache(profile: ImmutableProfile, file: string, manifest: ManifestV2) {
         try {
             const cacheDirectory: string = await this.initialiseCacheDirectory(manifest);
             const fileSafe = file.split("\\").join("/");
             await FsProvider.instance.copyFile(fileSafe, path.join(cacheDirectory, path.basename(fileSafe)));
             await this.writeManifestToCache(cacheDirectory, manifest);
-            await ProfileInstallerProvider.instance.uninstallMod(manifest, profile);
-            const profileInstallResult = await ProfileInstallerProvider.instance.installMod(manifest, profile);
-            if (profileInstallResult instanceof R2Error) {
-                callback(false, profileInstallResult);
-                return Promise.resolve();
-            }
-            const modListInstallResult = await ProfileModList.addMod(manifest, profile);
-            if (modListInstallResult instanceof R2Error) {
-                callback(false, modListInstallResult);
-                return Promise.resolve();
-            }
-            callback(true, null);
-            return Promise.resolve();
         } catch (e) {
-            callback(false, new R2Error("Error moving file to cache", (e as Error).message, null));
+            throw R2Error.fromThrownValue(e, "Error moving file to cache");
         }
+
+        await ProfileInstallerProvider.instance.uninstallMod(manifest, profile);
+        throwForR2Error(await ProfileInstallerProvider.instance.installMod(manifest, profile));
+        throwForR2Error(await ProfileModList.addMod(manifest, profile));
     }
 }

src/r2mm/installing/ZipExtract.ts

@@ -9,61 +9,46 @@ export default class ZipExtract {
 
     public static async extractAndDelete(zipFolder: string, filename: string, outputFolderName: string, callback: (success: boolean, error?: R2Error) => void): Promise<void> {
         const fs = FsProvider.instance;
-        return await this.extractOnly(path.join(zipFolder, filename), path.join(zipFolder, outputFolderName), async (result, err?) => {
-            if (result) {
-                try {
-                    await fs.unlink(path.join(zipFolder, filename));
-                    callback(result);
-                } catch (e) {
-                    const err: Error = e as Error;
-                    callback(result, new FileWriteError(
-                        'Failed to delete file',
-                        err.message,
-                        null
-                    ));
-                }
-            } else {
-                try {
-                    // Clear from cache as failed.
-                    await FileUtils.emptyDirectory(path.join(zipFolder, outputFolderName));
-                    await fs.rmdir(path.join(zipFolder, outputFolderName));
-                    await fs.unlink(path.join(zipFolder, filename));
-                    if (err !== undefined) {
-                        if (err instanceof R2Error) {
-                            callback(result, err);
-                        } else {
-                            throw err;
-                        }
-                    }
-                } catch (e) {
-                    // Cleanup might also fail e.g. for too long file paths on TSMM.
-                    // Show the original error instead of the one caused by the cleanup,
-                    // as the former is probably more informative for debugging.
-                    if (err) {
-                        callback(false, FileWriteError.fromThrownValue(err));
-                        return;
-                    }
+        const source = path.join(zipFolder, filename);
+        const destination = path.join(zipFolder, outputFolderName);
 
-                    callback(result, new FileWriteError(
-                        'Failed to extract zip',
-                        (e as Error).message,
-                        'Try to re-download the mod. If the issue persists, ask for help in the Thunderstore modding discord.'
-                    ));
-                }
+        try {
+            await this.extractOnly(source, destination);
+        } catch (originalError) {
+            try {
+                await FileUtils.emptyDirectory(destination);
+                await fs.rmdir(destination);
+                await fs.unlink(source);
+            } catch (cleanupError) {
+                // Cleanup might also fail e.g. for too long file paths on TSMM.
+                // Show the original error instead of the one caused by the cleanup,
+                // as the former is probably more informative for debugging.
+            } finally {
+                callback(false, FileWriteError.fromThrownValue(
+                    originalError instanceof R2Error ? originalError.message : `${originalError}`,
+                    `Failed to extract ${source}`,
+                    'Try to re-download the mod. If the issue persists, ask for help in the Thunderstore modding discord.'
+                ));
+                return;
             }
-        });
+        }
+
+        try {
+            await fs.unlink(source);
+        } catch (e) {
+            callback(false, FileWriteError.fromThrownValue(e, `Failed to delete ${source} after extraction`));
+            return;
+        }
+
+        callback(true);
     }
 
-    public static async extractOnly(zip: string, outputFolder: string, callback: (success: boolean, error?: Error) => void): Promise<void> {
+    public static async extractOnly(zip: string, outputFolder: string): Promise<void> {
         try {
             await ZipProvider.instance.extractAllTo(zip, outputFolder);
-            callback(true);
         } catch (e) {
             console.log("extractOnly failed:", e);
-            callback(false, new R2Error("Extraction failed", (e as Error).message, null));
+            throw R2Error.fromThrownValue(e, `Extracting ${zip} failed`);
         }
     }
-
-
-
 }

src/utils/ProfileUtils.ts

@@ -4,7 +4,7 @@ import * as yaml from "yaml";
 import { Store } from "vuex";
 
 import FileUtils from "./FileUtils";
-import R2Error from "../model/errors/R2Error";
+import R2Error, { throwForR2Error } from "../model/errors/R2Error";
 import ExportFormat from "../model/exports/ExportFormat";
 import ExportMod from "../model/exports/ExportMod";
 import Game from "../model/game/Game";
@@ -233,9 +233,3 @@ export async function readProfileFile(file: string): Promise<string> {
     }
     return read;
 }
-
-function throwForR2Error(maybeError: unknown) {
-    if (maybeError instanceof R2Error) {
-        throw maybeError;
-    }
-}