fix: don't restart on bad hardhat config (#212)

To stop a restart loop on bad hardhat config the hardhat worker has been
enhanced to have a lifecycle.

The background process starts as UNINITIALIZED. On `init` it moves to
`STARTING`. Any unexpected exits during starting moves it to
`INITIALIZATION_ERRORED`. Only an edit to the config file will trigger a
retry from this state.

On initialization completing within the background process, it will send
a `INITIALISATION_COMPLETE` message, that will move the state to
`RUNNING`.

An unexpected exit while `RUNNING` will lead to a restart.

Validation requests to the process if it is not in the running state (so
errored or still starting), leads to a validator error which will be
displayed to the user.

In response to #211.

server/src/services/validation/HardhatWorker.ts

@@ -1,70 +1,119 @@
-/* istanbul ignore file: wrapper around process */
-import * as path from "path";
 import * as childProcess from "child_process";
+import * as path from "path";
 import { HardhatProject } from "@analyzer/HardhatProject";
 import { Logger } from "@utils/Logger";
 import {
+  InitialisationCompleteMessage,
   InvalidatePreprocessingCacheMessage,
   ValidateCommand,
   ValidationCompleteMessage,
   WorkerProcess,
 } from "../../types";
 
+const UNINITIALIZED = "UNINITIALIZED";
+const STARTING = "STARTING";
+const RUNNING = "RUNNING";
+const INITIALIZATION_ERRORED = "INITIALIZATION_ERRORED";
+
+type HardhatWorkerStatus =
+  | typeof UNINITIALIZED
+  | typeof INITIALIZATION_ERRORED
+  | typeof STARTING
+  | typeof RUNNING;
+
+export function createProcessFor(
+  project: HardhatProject
+): childProcess.ChildProcess {
+  return childProcess.fork(path.resolve(__dirname, "worker.js"), {
+    cwd: project.basePath,
+    detached: true,
+  });
+}
+
 export class HardhatWorker implements WorkerProcess {
   public project: HardhatProject;
-  private child: childProcess.ChildProcess | null;
-  private logger: Logger;
-  private jobCount: number;
-
-  private jobs: {
+  public status: HardhatWorkerStatus;
+  public jobs: {
     [key: string]: {
       resolve: (message: ValidationCompleteMessage) => void;
       reject: (err: string) => void;
     };
   };
 
-  constructor(project: HardhatProject, logger: Logger) {
-    this.project = project;
+  private child: childProcess.ChildProcess | null;
+  private createProcessFor: (
+    project: HardhatProject
+  ) => childProcess.ChildProcess;
+  private logger: Logger;
+  private jobCount: number;
+
+  constructor(
+    project: HardhatProject,
+    givenCreateProcessFor: (
+      project: HardhatProject
+    ) => childProcess.ChildProcess,
+    logger: Logger
+  ) {
     this.child = null;
     this.jobCount = 0;
     this.jobs = {};
+
+    this.project = project;
+    this.createProcessFor = givenCreateProcessFor;
     this.logger = logger;
+
+    this.status = UNINITIALIZED;
   }
 
+  /**
+   * Setup the background validation process along with listeners
+   * on the LSP side.
+   *
+   * The status immediately moves from UNINITIALIZED -> STARTING. An
+   * `INITIALISATION_COMPLETE` message from the process will move
+   * the status to RUNNING (an unexpected exit will move it to
+   * INITIALIZATION_ERRORED).
+   */
   public init() {
-    this.child = childProcess.fork(path.resolve(__dirname, "worker.js"), {
-      cwd: this.project.basePath,
-      detached: true,
-    });
-
-    this.child.on("message", (message: ValidationCompleteMessage) => {
-      if (!(message.jobId in this.jobs)) {
-        this.logger.error("No job registered for validation complete");
-        return;
-      }
+    if (![UNINITIALIZED, INITIALIZATION_ERRORED].includes(this.status)) {
+      throw new Error("Cannot start a worker thread that has already started");
+    }
 
-      const { resolve } = this.jobs[message.jobId];
+    this.status = STARTING;
 
-      delete this.jobs[message.jobId];
+    this.child = this.createProcessFor(this.project);
 
-      resolve(message);
-    });
+    // deal with messages sent from the background process to the LSP
+    this.child.on(
+      "message",
+      (message: InitialisationCompleteMessage | ValidationCompleteMessage) => {
+        switch (message.type) {
+          case "INITIALISATION_COMPLETE":
+            this.status = RUNNING;
+            this.logger.trace(
+              `initialisation complete for ${this.project.basePath}`
+            );
+            break;
+          case "VALIDATION_COMPLETE":
+            this._validationComplete(message);
+            break;
+          default:
+            this._unexectpedMessage(message);
+            break;
+        }
+      }
+    );
 
+    // errors on the background thread are logged
     this.child.on("error", (err) => {
       this.logger.error(err);
     });
 
-    this.child.on("exit", (code, signal) => {
-      this.logger.trace(
-        `Hardhat Worker Process restart (${code}): ${this.project.basePath}`
-      );
-
-      if (code === 0 || signal !== null) {
-        return;
-      }
-
-      return this.restart();
-    });
+    // if the background process exits due to an error
+    // we restart if it has previously been running,
+    // if exits during initialization, we leave it in
+    // the errored state
+    this.child.on("exit", this.handleExit.bind(this));
   }
 
   public async validate({
@@ -88,6 +137,14 @@ export class HardhatWorker implements WorkerProcess {
         return reject(new Error("No child process to send validation"));
       }
 
+      if (this.status !== RUNNING) {
+        return this._validationBlocked(
+          { jobId, projectBasePath },
+          resolve,
+          reject
+        );
+      }
+
       this.jobs[jobId] = { resolve, reject };
 
       const message: ValidateCommand = {
@@ -108,6 +165,12 @@ export class HardhatWorker implements WorkerProcess {
     });
   }
 
+  /**
+   * Inform the background validation process to clear its file caches
+   * and reread the solidity files from disk on the next job.
+   *
+   * @returns whether the cace was cleared
+   */
   public async invalidatePreprocessingCache(): Promise<boolean> {
     return new Promise((resolve, reject) => {
       if (this.child === null) {
@@ -116,6 +179,11 @@ export class HardhatWorker implements WorkerProcess {
         );
       }
 
+      // Only running validators can have their cache cleared
+      if (this.status !== RUNNING) {
+        return resolve(false);
+      }
+
       const message: InvalidatePreprocessingCacheMessage = {
         type: "INVALIDATE_PREPROCESSING_CACHE",
       };
@@ -130,19 +198,122 @@ export class HardhatWorker implements WorkerProcess {
     });
   }
 
+  /**
+   * Stop the current background validation process.
+   *
+   * The status will be set back to the unstarted state (UNINITIALIZED).
+   */
   public kill() {
     this.child?.kill();
+    // reset status to allow restarting in future
+    this.status = UNINITIALIZED;
   }
 
+  /**
+   * Stop the current background validation process and start a new one.
+   *
+   * The jobs being currently processeded are all cancelled.
+   */
   public async restart(): Promise<void> {
+    this.logger.trace(`Restarting hardhat worker for ${this.project.basePath}`);
+
+    this._cancelCurrentJobs();
+
+    this.kill();
+    this.init();
+  }
+
+  public handleExit(code: number | null, signal: NodeJS.Signals | null) {
+    this.logger.trace(
+      `Hardhat Worker Process restart (${code}): ${this.project.basePath}`
+    );
+
+    if (code === 0 || signal !== null) {
+      this.status = UNINITIALIZED;
+      this._cancelCurrentJobs();
+      return;
+    }
+
+    if (this.status === STARTING) {
+      this.status = INITIALIZATION_ERRORED;
+      this._cancelCurrentJobs();
+      return;
+    }
+
+    if (this.status !== RUNNING) {
+      this.logger.error(
+        new Error(
+          "Exit from validator that is already UNINITIALIZED/INITIALIZATION_ERRORED"
+        )
+      );
+      return;
+    }
+
+    return this.restart();
+  }
+
+  private _validationComplete(message: ValidationCompleteMessage) {
+    if (!(message.jobId in this.jobs)) {
+      this.logger.error("No job registered for validation complete");
+      return;
+    }
+
+    const { resolve } = this.jobs[message.jobId];
+
+    delete this.jobs[message.jobId];
+
+    resolve(message);
+  }
+
+  private _unexectpedMessage(message: never) {
+    this.logger.error(new Error(`Unexpected error type: ${message}`));
+  }
+
+  private _validationBlocked(
+    { jobId, projectBasePath }: { jobId: number; projectBasePath: string },
+    resolve: (
+      value: ValidationCompleteMessage | PromiseLike<ValidationCompleteMessage>
+    ) => void,
+    _reject: (reason?: string) => void
+  ): void {
+    if (this.status === STARTING) {
+      return resolve({
+        type: "VALIDATION_COMPLETE",
+        status: "VALIDATOR_ERROR",
+        jobId,
+        projectBasePath,
+        reason: "validator-starting",
+      });
+    }
+
+    if (this.status === "INITIALIZATION_ERRORED") {
+      return resolve({
+        type: "VALIDATION_COMPLETE",
+        status: "VALIDATOR_ERROR",
+        jobId,
+        projectBasePath,
+        reason: "validator-initialization-failed",
+      });
+    }
+
+    return resolve({
+      type: "VALIDATION_COMPLETE",
+      status: "VALIDATOR_ERROR",
+      jobId,
+      projectBasePath,
+      reason: "validator-in-unexpected-state",
+    });
+  }
+
+  /**
+   * Reject any open jobs whose result is still promised.
+   */
+  private _cancelCurrentJobs() {
     for (const jobId of Object.keys(this.jobs)) {
       const { reject } = this.jobs[jobId];
       reject("Worker process restarted");
 
       delete this.jobs[jobId];
     }
-
-    this.kill();
-    this.init();
   }
 }

server/src/services/validation/compilerProcessFactory.ts

@@ -2,11 +2,11 @@
 import { HardhatProject } from "@analyzer/HardhatProject";
 import { Logger } from "@utils/Logger";
 import { WorkerProcess } from "../../types";
-import { HardhatWorker } from "./HardhatWorker";
+import { createProcessFor, HardhatWorker } from "./HardhatWorker";
 
 export function compilerProcessFactory(
   project: HardhatProject,
   logger: Logger
 ): WorkerProcess {
-  return new HardhatWorker(project, logger);
+  return new HardhatWorker(project, createProcessFor, logger);
 }