From 865e89301999369320f5302e5587635e6f6d4d26 Mon Sep 17 00:00:00 2001
From: Sebastian Malton <sebastian@malton.name>
Date: Wed, 14 Apr 2021 16:23:38 -0400
Subject: [PATCH] better use of disposers

Signed-off-by: Sebastian Malton <sebastian@malton.name>
---
 src/common/utils/disposer.ts                  |  4 +-
 .../+extensions/extension-install.store.ts    |  4 +-
 .../components/+extensions/extensions.tsx     | 38 ++++++++-----------
 3 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/src/common/utils/disposer.ts b/src/common/utils/disposer.ts
index 93c404015b..5e26bcd0e1 100644
--- a/src/common/utils/disposer.ts
+++ b/src/common/utils/disposer.ts
@@ -4,7 +4,9 @@ interface Extendable<T> {
   push(...vals: T[]): void;
 }
 
-export function disposer(...args: Disposer[]): Disposer & Extendable<Disposer> {
+export type ExtendableDisposer = Disposer & Extendable<Disposer>;
+
+export function disposer(...args: Disposer[]): ExtendableDisposer {
   const res = () => {
     args.forEach(dispose => dispose?.());
     args.length = 0;
diff --git a/src/renderer/components/+extensions/extension-install.store.ts b/src/renderer/components/+extensions/extension-install.store.ts
index 3bbe53e5ea..787dc2b364 100644
--- a/src/renderer/components/+extensions/extension-install.store.ts
+++ b/src/renderer/components/+extensions/extension-install.store.ts
@@ -1,6 +1,6 @@
 import { action, computed, observable } from "mobx";
 import logger from "../../../main/logger";
-import { disposer, Disposer } from "../../utils";
+import { disposer, ExtendableDisposer } from "../../utils";
 import * as uuid from "uuid";
 import { broadcastMessage } from "../../../common/ipc";
 import { ipcRenderer } from "electron";
@@ -76,7 +76,7 @@ export class ExtensionInstallationStateStore {
    * determined.
    * @returns a disposer which should be called to mark the end of the install phase
    */
-  @action static startPreInstall(): Disposer {
+  @action static startPreInstall(): ExtendableDisposer {
     const preInstallStepId = uuid.v4();
 
     logger.debug(`${Prefix}: starting a new preinstall phase: ${preInstallStepId}`);
diff --git a/src/renderer/components/+extensions/extensions.tsx b/src/renderer/components/+extensions/extensions.tsx
index 87204e985b..5142c92eef 100644
--- a/src/renderer/components/+extensions/extensions.tsx
+++ b/src/renderer/components/+extensions/extensions.tsx
@@ -6,7 +6,7 @@ import { disposeOnUnmount, observer } from "mobx-react";
 import os from "os";
 import path from "path";
 import React from "react";
-import { autobind, disposer, Disposer, downloadFile, downloadJson, extractTar, listTarEntries, noop, readFileFromTar } from "../../../common/utils";
+import { autobind, disposer, Disposer, downloadFile, downloadJson, ExtendableDisposer, extractTar, listTarEntries, noop, readFileFromTar } from "../../../common/utils";
 import { docsUrl } from "../../../common/vars";
 import { extensionDiscovery, InstalledExtension, manifestFilename } from "../../../extensions/extension-discovery";
 import { extensionLoader } from "../../../extensions/extension-loader";
@@ -166,13 +166,15 @@ async function validatePackage(filePath: string): Promise<LensExtensionManifest>
   return manifest;
 }
 
-async function createTempFilesAndValidate({ fileName, dataP }: InstallRequest, { showErrors = true } = {}): Promise<InstallRequestValidated | null> {
+async function createTempFilesAndValidate({ fileName, dataP }: InstallRequest, disposer: ExtendableDisposer): Promise<InstallRequestValidated | null> {
   // copy files to temp
   await fse.ensureDir(getExtensionPackageTemp());
 
   // validate packages
   const tempFile = getExtensionPackageTemp(fileName);
 
+  disposer.push(() => fse.unlink(tempFile));
+
   try {
     const data = await dataP;
 
@@ -192,19 +194,15 @@ async function createTempFilesAndValidate({ fileName, dataP }: InstallRequest, {
       id,
     };
   } catch (error) {
-    fse.unlink(tempFile).catch(noop); // remove invalid temp package
+    const message = getMessageFromError(error);
 
-    if (showErrors) {
-      const message = getMessageFromError(error);
-
-      logger.info(`[EXTENSION-INSTALLATION]: installing ${fileName} has failed: ${message}`, { error });
-      Notifications.error(
-        <div className="flex column gaps">
-          <p>Installing <em>{fileName}</em> has failed, skipping.</p>
-          <p>Reason: <em>{message}</em></p>
-        </div>
-      );
-    }
+    logger.info(`[EXTENSION-INSTALLATION]: installing ${fileName} has failed: ${message}`, { error });
+    Notifications.error(
+      <div className="flex column gaps">
+        <p>Installing <em>{fileName}</em> has failed, skipping.</p>
+        <p>Reason: <em>{message}</em></p>
+      </div>
+    );
   }
 
   return null;
@@ -317,9 +315,9 @@ export async function attemptInstallByInfo({ name, version, requireConfirmation
   return attemptInstall({ fileName, dataP }, disposer);
 }
 
-async function attemptInstall(request: InstallRequest, d?: Disposer): Promise<void> {
+async function attemptInstall(request: InstallRequest, d?: ExtendableDisposer): Promise<void> {
   const dispose = disposer(ExtensionInstallationStateStore.startPreInstall(), d);
-  const validatedRequest = await createTempFilesAndValidate(request);
+  const validatedRequest = await createTempFilesAndValidate(request, dispose);
 
   if (!validatedRequest) {
     return dispose();
@@ -366,15 +364,11 @@ async function attemptInstall(request: InstallRequest, d?: Disposer): Promise<vo
             await unpackExtension(validatedRequest, dispose);
           } else {
             dispose();
-            await fse.unlink(validatedRequest.tempFile).catch(noop);
           }
         }} />
       </div>,
       {
-        onClose() {
-          dispose();
-          fse.unlink(validatedRequest.tempFile).catch(noop);
-        }
+        onClose: dispose,
       }
     );
   }
@@ -399,7 +393,7 @@ async function installOnDrop(files: File[]) {
 }
 
 async function installFromInput(input: string) {
-  let disposer: Disposer;
+  let disposer: ExtendableDisposer | undefined = undefined;
 
   try {
     // fixme: improve error messages for non-tar-file URLs