Make elm invocations killable (#16)

src/Env.ts

@@ -49,3 +49,9 @@ export const __ELM_WATCH_MAX_PARALLEL = "__ELM_WATCH_MAX_PARALLEL";
 
 // Used to test ElmWatchDummy.elm errors without affecting other tests.
 export const __ELM_WATCH_TMP_DIR = "__ELM_WATCH_TMP_DIR";
+
+// When killing `elm`, we don’t do it right away. Instead, we give it a chance
+// to finish. The thinking is that this can reduce corruption of elm-stuff.
+// Used in tests by not having to wait for so long.
+// Type: Number.
+export const __ELM_WATCH_ELM_TIMEOUT = "__ELM_WATCH_ELM_TIMEOUT";

src/Hot.ts

@@ -118,6 +118,7 @@ type Mutable = {
   lastInfoMessage: string | undefined;
   watcherTimeoutId: NodeJS.Timeout | undefined;
   elmWatchStuffJsonWriteError: Error | undefined;
+  killInstallDependencies: (() => void) | undefined;
 };
 
 type WebSocketConnection = {
@@ -155,6 +156,7 @@ type Msg =
     }
   | {
       tag: "InstallDependenciesDone";
+      date: Date;
       installResult: Compile.InstallDependenciesResult;
     }
   | {
@@ -258,6 +260,7 @@ type Cmd =
         outputPath: OutputPath;
         outputState: OutputState;
       }>;
+      killInstallDependencies: boolean;
     }
   | {
       tag: "NoCmd";
@@ -484,6 +487,7 @@ const initMutable =
       lastInfoMessage: undefined,
       watcherTimeoutId: undefined,
       elmWatchStuffJsonWriteError: undefined,
+      killInstallDependencies: undefined,
     };
 
     webSocketServer.setDispatch((msg) => {
@@ -520,20 +524,16 @@ const initMutable =
 
       // istanbul ignore next
       try {
+        if (mutable.killInstallDependencies !== undefined) {
+          mutable.killInstallDependencies();
+        }
         await Promise.all(
           getFlatOutputs(project).map(({ outputState }) =>
-            outputState.status.tag === "Postprocess"
+            "kill" in outputState.status
               ? outputState.status.kill()
               : Promise.resolve()
           )
         );
-      } catch (unknownError) {
-        const error = toError(unknownError);
-        rejectPromise(toError(error));
-      }
-
-      // istanbul ignore next
-      try {
         await closeAll(mutable);
       } catch (unknownError) {
         const error = toError(unknownError);
@@ -826,6 +826,11 @@ function update(
                 ],
               ];
 
+            // We only kill installing dependencies when a restart is needed.
+            // Wait for the restart to happen.
+            case "Killed":
+              return [{ ...model, hotState: { tag: "Idle" } }, []];
+
             case "Success": {
               return [
                 {
@@ -1098,6 +1103,7 @@ function onWatcherEvent(
                   {
                     tag: "MarkAsDirty",
                     outputs: getFlatOutputs(project),
+                    killInstallDependencies: false,
                   },
                   { tag: "RestartWorkers" },
                 ],
@@ -1150,7 +1156,13 @@ function onElmFileWatcherEvent(
     ? [
         compileNextAction(nextAction),
         { ...event, affectsAnyTarget: true },
-        [{ tag: "MarkAsDirty", outputs: dirtyOutputs }],
+        [
+          {
+            tag: "MarkAsDirty",
+            outputs: dirtyOutputs,
+            killInstallDependencies: false,
+          },
+        ],
       ]
     : [nextAction, { ...event, affectsAnyTarget: false }, []];
 }
@@ -1361,15 +1373,35 @@ const runCmd =
         }
         return;
 
-      case "InstallDependencies":
+      case "InstallDependencies": {
         // If the web socket server fails to boot, don’t even bother with anything else.
         mutable.webSocketServer.listening
-          .then(() => Compile.installDependencies(env, logger, mutable.project))
+          .then(() => {
+            const { promise, kill } = Compile.installDependencies(
+              env,
+              logger,
+              getNow,
+              mutable.project
+            );
+            mutable.killInstallDependencies = () => {
+              kill();
+              mutable.killInstallDependencies = undefined;
+            };
+            return promise;
+          })
+          .finally(() => {
+            mutable.killInstallDependencies = undefined;
+          })
           .then((installResult) => {
-            dispatch({ tag: "InstallDependenciesDone", installResult });
+            dispatch({
+              tag: "InstallDependenciesDone",
+              date: getNow(),
+              installResult,
+            });
           })
           .catch(rejectPromise);
         return;
+      }
 
       case "LimitWorkers":
         mutable.postprocessWorkerPool
@@ -1420,10 +1452,16 @@ const runCmd =
       }
 
       case "MarkAsDirty":
+        if (
+          cmd.killInstallDependencies &&
+          mutable.killInstallDependencies !== undefined
+        ) {
+          mutable.killInstallDependencies();
+        }
         for (const { outputPath, outputState } of cmd.outputs) {
           outputState.dirty = true;
-          if (outputState.status.tag === "Postprocess") {
-            outputState.status.kill().catch(rejectPromise);
+          if ("kill" in outputState.status) {
+            Promise.resolve(outputState.status.kill()).catch(rejectPromise);
           }
           webSocketSendToOutput(
             outputPath,
@@ -1773,6 +1811,7 @@ function makeRestartNextAction(
         // Interrupt all compilation.
         tag: "MarkAsDirty",
         outputs: getFlatOutputs(project),
+        killInstallDependencies: true,
       },
     ],
   ];
@@ -2213,6 +2252,7 @@ function onWebSocketRecompileNeeded(
           {
             tag: "MarkAsDirty",
             outputs: [{ outputPath, outputState }],
+            killInstallDependencies: false,
           },
         ],
       ];

src/Make.ts

@@ -19,10 +19,16 @@ export async function run(
 ): Promise<MakeResult> {
   const startTimestamp = getNow().getTime();
 
-  const installResult = await Compile.installDependencies(env, logger, project);
+  const installResult = await Compile.installDependencies(
+    env,
+    logger,
+    getNow,
+    project
+  ).promise;
 
   switch (installResult.tag) {
     case "Error":
+    case "Killed":
       return { tag: "Error" };
 
     case "Success":

src/Postprocess.ts

@@ -12,7 +12,7 @@ import {
   MessageToWorker,
   PostprocessResult,
 } from "./PostprocessShared";
-import { Command, spawnKillable, SpawnResult } from "./Spawn";
+import { Command, spawn, SpawnResult } from "./Spawn";
 import {
   CompilationMode,
   ElmWatchJsonPath,
@@ -74,7 +74,7 @@ export function runPostprocess({
     stdin: code,
   };
 
-  const { promise, kill } = spawnKillable(command);
+  const { promise, kill } = spawn(command);
 
   const handleSpawnResult = (spawnResult: SpawnResult): PostprocessResult => {
     switch (spawnResult.tag) {
@@ -89,6 +89,9 @@ export function runPostprocess({
           command: spawnResult.command,
         };
 
+      case "Killed":
+        return { tag: "Killed" };
+
       case "Exit": {
         const { exitReason } = spawnResult;
 
@@ -188,10 +191,6 @@ type PostprocessWorkerStatus =
       tag: "Terminated";
     };
 
-export const WORKER_TERMINATED = new Error(
-  "`PostprocessWorker` has a `terminate` method. That was called! This error is supposed to be caught."
-);
-
 class PostprocessWorker {
   private worker = new Worker(path.join(__dirname, "PostprocessWorker.js"), {
     stdout: true,
@@ -338,11 +337,11 @@ class PostprocessWorker {
         break;
 
       case "Busy": {
-        const { reject } = this.status;
+        const { resolve } = this.status;
         this.status = { tag: "Terminated" };
         this.onTerminated(this);
         await this.worker.terminate();
-        reject(WORKER_TERMINATED);
+        resolve({ tag: "Killed" });
         break;
       }
 

src/PostprocessShared.ts

@@ -39,6 +39,9 @@ export type MessageFromWorker = {
 
 export type PostprocessResult =
   | PostprocessError
+  | {
+      tag: "Killed";
+    }
   | {
       tag: "Success";
       code: Buffer | string;

src/Project.ts

@@ -149,11 +149,13 @@ export type OutputStatus =
       elmDurationMs: number;
       walkerDurationMs: number;
       injectDurationMs: number;
+      kill: () => void;
     }
   | {
       tag: "ElmMakeTypecheckOnly";
       elmDurationMs: number;
       walkerDurationMs: number;
+      kill: () => void;
     }
   | {
       tag: "Interrupted";
@@ -163,7 +165,7 @@ export type OutputStatus =
     }
   | {
       tag: "Postprocess";
-      kill: () => Promise<void>;
+      kill: () => Promise<void> | void;
     }
   | {
       tag: "QueuedForElmMake";

src/Spawn.ts

@@ -16,6 +16,10 @@ export type SpawnResult =
       stderr: Buffer;
       command: Command;
     }
+  | {
+      tag: "Killed";
+      command: Command;
+    }
   | {
       tag: "OtherSpawnError";
       error: Error;
@@ -37,15 +41,7 @@ export type Command = {
   stdin?: Buffer | string;
 };
 
-export const SPAWN_KILLED = new Error(
-  "`spawnKillable` returns a `kill` function. That was called! This error is supposed to be caught."
-);
-
-export async function spawn(command: Command): Promise<SpawnResult> {
-  return spawnKillable(command).promise;
-}
-
-export function spawnKillable(command: Command): {
+export function spawn(command: Command): {
   promise: Promise<SpawnResult>;
   kill: () => void;
 } {
@@ -59,10 +55,10 @@ export function spawnKillable(command: Command): {
   const promise = (
     actualSpawn: typeof childProcess.spawn
   ): Promise<SpawnResult> =>
-    new Promise((resolve, reject) => {
+    new Promise((resolve) => {
       // istanbul ignore if
       if (killed) {
-        reject(SPAWN_KILLED);
+        resolve({ tag: "Killed", command });
         return;
       }
 
@@ -159,7 +155,7 @@ export function spawnKillable(command: Command): {
         // istanbul ignore else
         if (!killed) {
           child.kill();
-          reject(SPAWN_KILLED);
+          resolve({ tag: "Killed", command });
           killed = true;
         }
       };