fix: improve file-parse error logs (#345)

Improve the typing and logs for file-parse errors.

src/cli/error-logging.ts

@@ -12,18 +12,26 @@ import { NoWslError } from "../io/wsl";
 import { ChildProcessError } from "../utils/process";
 import { RequiredEnvMissingError } from "../io/upm-config-io";
 import { FileMissingError, GenericIOError } from "../io/common-errors";
+import { StringFormatError } from "../utils/string-parsing";
 
 /**
  * Logs a {@link ManifestLoadError} to the console.
  */
 export function logManifestLoadError(log: Logger, error: ManifestLoadError) {
+  const reason =
+    error instanceof FileMissingError
+      ? `it could not be found at "${error.path}"`
+      : error instanceof GenericIOError
+      ? "a file-system interaction failed"
+      : error instanceof StringFormatError
+      ? "the manifest file did not contain valid json"
+      : "the manifest file did not contain a valid project manifest";
+
   const prefix = "manifest";
-  if (error instanceof FileMissingError)
-    log.error(prefix, `manifest at ${error.path} does not exist`);
-  else if (error instanceof FileParseError) {
-    log.error(prefix, `failed to parse manifest at ${error.path}`);
-    if (error.cause !== undefined) log.error(prefix, error.cause.message);
-  }
+  const errorMessage = `Could not load project manifest because ${reason}.`;
+  log.error(prefix, errorMessage);
+
+  // TODO: Print fix suggestions
 }
 
 /**
@@ -69,7 +77,7 @@ export function logEnvParseError(log: Logger, error: EnvParseError) {
       : error instanceof ChildProcessError
       ? "a required child process failed"
       : error instanceof FileParseError
-      ? `the file at ${error.path} could not parsed into a ${error.targetDescription}`
+      ? `the project version file (ProjectVersion.txt) has an invalid structure`
       : error instanceof RequiredEnvMissingError
       ? `none of the following environment variables were set: ${error.keyNames.join(
           ", "

src/common-errors.ts

@@ -1,6 +1,5 @@
 import { CustomError } from "ts-custom-error";
 import { EditorVersion } from "./domain/editor-version";
-import { StringFormatError } from "./utils/string-parsing";
 
 /**
  * Error for when the packument was not found.
@@ -27,21 +26,18 @@ export class PackageWithVersionError extends CustomError {
 /**
  * Error for when a file could not be parsed into a specific target type.
  */
-export class FileParseError extends CustomError {
+export class FileParseError<const TTarget extends string> extends CustomError {
   constructor(
     /**
      * The path to the file that could not be parsed.
      */
-    readonly path: string,
+    public readonly filePath: string,
     /**
      * A description or name of the thing that the file was supposed to be
-     * parsed to.
+     * parsed to. This should be a constant string that can be used in ifs
+     * and switches.
      */
-    readonly targetDescription: string,
-    /**
-     * The error that caused this one.
-     */
-    readonly cause?: StringFormatError
+    public readonly targetDescription: TTarget
   ) {
     super("A file could not be parsed into a specific target type.");
   }

src/io/project-manifest-io.ts

@@ -3,7 +3,7 @@ import {
   UnityProjectManifest,
 } from "../domain/project-manifest";
 import path from "path";
-import { AsyncResult } from "ts-results-es";
+import { AsyncResult, Err, Ok } from "ts-results-es";
 import { ReadTextFile, WriteTextFile } from "./fs-result";
 import { StringFormatError, tryParseJson } from "../utils/string-parsing";
 import { FileParseError } from "../common-errors";
@@ -33,13 +33,29 @@ export function makeProjectManifestMissingError(
   return new FileMissingError("ProjectManifest", filePath);
 }
 
+/**
+ * Error for when the project manifest could not be parsed.
+ */
+export type ProjectManifestParseError = FileParseError<"ProjectManifest">;
+
+/**
+ * Makes a {@link ProjectManifestParseError} object.
+ * @param filePath The path of the file.
+ */
+export function makeProjectManifestParseError(
+  filePath: string
+): ProjectManifestParseError {
+  return new FileParseError(filePath, "ProjectManifest");
+}
+
 /**
  * Error which may occur when loading a project manifest.
  */
 export type ManifestLoadError =
   | ProjectManifestMissingError
   | GenericIOError
-  | FileParseError;
+  | StringFormatError
+  | ProjectManifestParseError;
 
 /**
  * Function for loading the project manifest for a Unity project.
@@ -57,22 +73,19 @@ export function makeProjectManifestLoader(
 ): LoadProjectManifest {
   return (projectPath) => {
     const manifestPath = manifestPathFor(projectPath);
-    return (
-      readFile(manifestPath)
-        .mapErr((error) =>
-          error.code === "ENOENT"
-            ? makeProjectManifestMissingError(manifestPath)
-            : new GenericIOError()
-        )
-        .andThen(tryParseJson)
-        // TODO: Actually validate the json structure
-        .map((json) => json as unknown as UnityProjectManifest)
-        .mapErr((error) =>
-          error instanceof StringFormatError
-            ? new FileParseError(manifestPath, "Project-manifest", error)
-            : error
-        )
-    );
+    return readFile(manifestPath)
+      .mapErr((error) =>
+        error.code === "ENOENT"
+          ? makeProjectManifestMissingError(manifestPath)
+          : new GenericIOError()
+      )
+      .andThen(tryParseJson)
+      .andThen((json) =>
+        typeof json === "object"
+          ? // TODO: Actually validate the json structure
+            Ok(json as unknown as UnityProjectManifest)
+          : Err(makeProjectManifestParseError(manifestPath))
+      );
   };
 }
 

src/io/project-version-io.ts

@@ -24,14 +24,29 @@ export function makeProjectVersionMissingError(
   return new FileMissingError("ProjectVersion.txt", filePath);
 }
 
+/**
+ * Error for when the project version could not be parsed.
+ */
+export type ProjectVersionParseError = FileParseError<"ProjectVersion.txt">;
+
+/**
+ * Makes a {@link ProjectVersionParseError} object.
+ * @param filePath The path of the file.
+ */
+export function makeProjectVersionParseError(
+  filePath: string
+): ProjectVersionParseError {
+  return new FileParseError(filePath, "ProjectVersion.txt");
+}
+
 /**
  * Error which may occur when loading a project-version.
  */
 export type ProjectVersionLoadError =
-  | StringFormatError
-  | FileParseError
   | ProjectVersionMissingError
-  | GenericIOError;
+  | GenericIOError
+  | StringFormatError
+  | ProjectVersionParseError;
 
 /**
  * Function for loading a projects editor version string.
@@ -67,7 +82,7 @@ export function makeProjectVersionLoader(
             typeof content.m_EditorVersion === "string"
           )
         )
-          return Err(new FileParseError(filePath, "Project-version"));
+          return Err(makeProjectVersionParseError(filePath));
 
         return Ok(content.m_EditorVersion);
       });

src/services/parse-env.ts

@@ -158,15 +158,8 @@ export function makeParseEnvService(
 
     // editor version
     const projectVersionLoadResult = await loadProjectVersion(cwd).promise;
-    if (projectVersionLoadResult.isErr()) {
-      const error = projectVersionLoadResult.error;
-      if (error instanceof FileParseError)
-        log.error(
-          "ProjectVersion",
-          "ProjectVersion.txt could not be parsed for editor-version!"
-        );
-      return projectVersionLoadResult;
-    }
+    if (projectVersionLoadResult.isErr()) return projectVersionLoadResult;
+
     const unparsedEditorVersion = projectVersionLoadResult.value;
     const parsedEditorVersion = tryParseEditorVersion(unparsedEditorVersion);
     const editorVersion =

test/io/project-manifest-io.test.ts

@@ -8,7 +8,6 @@ import {
   mapScopedRegistry,
 } from "../../src/domain/project-manifest";
 import path from "path";
-import { FileParseError } from "../../src/common-errors";
 import { ReadTextFile, WriteTextFile } from "../../src/io/fs-result";
 import { Err, Ok } from "ts-results-es";
 import { buildProjectManifest } from "../domain/data-project-manifest";
@@ -18,6 +17,7 @@ import { exampleRegistryUrl } from "../domain/data-registry";
 import { mockService } from "../services/service.mock";
 import { eaccesError, enoentError } from "./node-error.mock";
 import { FileMissingError, GenericIOError } from "../../src/io/common-errors";
+import { StringFormatError } from "../../src/utils/string-parsing";
 
 const exampleProjectPath = "/some/path";
 describe("project-manifest io", () => {
@@ -72,7 +72,7 @@ describe("project-manifest io", () => {
       const result = await loadProjectManifest(exampleProjectPath).promise;
 
       expect(result).toBeError((actual) =>
-        expect(actual).toBeInstanceOf(FileParseError)
+        expect(actual).toBeInstanceOf(StringFormatError)
       );
     });
 

test/services/parse-env.test.ts

@@ -3,7 +3,6 @@ import { NpmAuth } from "another-npm-registry-client";
 import { Env, makeParseEnvService } from "../../src/services/parse-env";
 import { Err, Ok } from "ts-results-es";
 import { GetUpmConfigPath, LoadUpmConfig } from "../../src/io/upm-config-io";
-import { FileParseError } from "../../src/common-errors";
 import { makeEditorVersion } from "../../src/domain/editor-version";
 import { NoWslError } from "../../src/io/wsl";
 import { mockUpmConfig } from "../io/upm-config-io.mock";
@@ -522,26 +521,5 @@ describe("env", () => {
 
       expect(result).toBeError((error) => expect(error).toEqual(expected));
     });
-
-    it("should notify of parsing issue", async () => {
-      const { parseEnv, log, loadProjectVersion } = makeDependencies();
-      loadProjectVersion.mockReturnValue(
-        Err(
-          new FileParseError(
-            "/some/path/ProjectVersion.txt",
-            "ProjectVersion.txt"
-          )
-        ).toAsyncResult()
-      );
-
-      await parseEnv({
-        _global: {},
-      });
-
-      expect(log.error).toHaveBeenCalledWith(
-        "ProjectVersion",
-        expect.stringContaining("could not be parsed")
-      );
-    });
   });
 });