feat: improve error messages (#347)

* test: improve console matcher Allow using expect matcher for prefix * add-cmd * deps-cmd * login-cmd * remove-cmd * search-cmd * view-cmd * rename variables * exception to log * use jest assertion * fix missing error class props * better network errors * add more error messages
openupm · May 30, 2024 · 069970e · 069970e
1 parent f66625a
commit 069970e
Show file tree

Hide file tree

Showing 33 changed files with 794 additions and 531 deletions.
diff --git a/src/cli/cmd-add.ts b/src/cli/cmd-add.ts
@@ -1,11 +1,9 @@
 import { isPackageUrl, PackageUrl } from "../domain/package-url";
 import {
   LoadProjectManifest,
-  ManifestLoadError,
-  ManifestWriteError,
   WriteProjectManifest,
 } from "../io/project-manifest-io";
-import { EnvParseError, ParseEnvService } from "../services/parse-env";
+import { ParseEnvService } from "../services/parse-env";
 import {
   compareEditorVersion,
   stringifyEditorVersion,
@@ -36,13 +34,6 @@ import { SemanticVersion } from "../domain/semantic-version";
 import { areArraysEqual } from "../utils/array-utils";
 import { Err, Ok, Result } from "ts-results-es";
 import { CustomError } from "ts-custom-error";
-import {
-  logDetermineEditorError,
-  logEnvParseError,
-  logManifestLoadError,
-  logManifestSaveError,
-  logPackumentResolveError,
-} from "./error-logging";
 import {
   DependencyResolveError,
   ResolveDependenciesService,
@@ -53,9 +44,17 @@ import { logValidDependency } from "./dependency-logging";
 import { unityRegistryUrl } from "../domain/registry-url";
 import { tryGetTargetEditorVersionFor } from "../domain/package-manifest";
 import { VersionNotFoundError } from "../domain/packument";
-import { FetchPackumentError } from "../io/packument-io";
 import { DebugLog } from "../logging";
 import { DetermineEditorVersion } from "../services/determine-editor-version";
+import { FetchPackumentError } from "../io/packument-io";
+import { ResultCodes } from "./result-codes";
+import {
+  notifyEnvParsingFailed,
+  notifyManifestLoadFailed,
+  notifyManifestWriteFailed,
+  notifyProjectVersionLoadFailed,
+  notifyRemotePackumentVersionResolvingFailed,
+} from "./error-logging";
 
 export class InvalidPackumentDataError extends CustomError {
   private readonly _class = "InvalidPackumentDataError";
@@ -85,16 +84,18 @@ export type AddOptions = CmdOptions<{
   force?: boolean;
 }>;
 
-export type AddError =
-  | EnvParseError
-  | ManifestLoadError
+type AddError =
   | PackumentVersionResolveError
   | FetchPackumentError
   | InvalidPackumentDataError
   | EditorIncompatibleError
   | UnresolvedDependencyError
-  | DependencyResolveError
-  | ManifestWriteError;
+  | DependencyResolveError;
+
+/**
+ * The different command result codes for the add command.
+ */
+export type AddResultCode = ResultCodes.Ok | ResultCodes.Error;
 
 /**
  * Cmd-handler for adding packages.
@@ -104,7 +105,7 @@ export type AddError =
 type AddCmd = (
   pkgs: PackageReference | PackageReference[],
   options: AddOptions
-) => Promise<Result<void, AddError>>;
+) => Promise<AddResultCode>;
 
 /**
  * Makes a {@link AddCmd} function.
@@ -121,18 +122,19 @@ export function makeAddCmd(
 ): AddCmd {
   return async (pkgs, options) => {
     if (!Array.isArray(pkgs)) pkgs = [pkgs];
+
     // parse env
     const envResult = await parseEnv(options);
     if (envResult.isErr()) {
-      logEnvParseError(log, envResult.error);
-      return envResult;
+      notifyEnvParsingFailed(log, envResult.error);
+      return ResultCodes.Error;
     }
     const env = envResult.value;
 
     const editorVersionResult = await determineEditorVersion(env.cwd).promise;
     if (editorVersionResult.isErr()) {
-      logDetermineEditorError(log, editorVersionResult.error);
-      return editorVersionResult;
+      notifyProjectVersionLoadFailed(log, editorVersionResult.error);
+      return ResultCodes.Error;
     }
     const editorVersion = editorVersionResult.value;
 
@@ -175,7 +177,11 @@ export function makeAddCmd(
         }
 
         if (resolveResult.isErr()) {
-          logPackumentResolveError(log, name, resolveResult.error);
+          notifyRemotePackumentVersionResolvingFailed(
+            log,
+            name,
+            resolveResult.error
+          );
           return resolveResult;
         }
 
@@ -232,10 +238,14 @@ export function makeAddCmd(
             requestedVersion,
             true
           );
-          if (resolveResult.isErr())
-            // TODO: Log errors
-            // TODO: Add tests
+          if (resolveResult.isErr()) {
+            notifyRemotePackumentVersionResolvingFailed(
+              log,
+              name,
+              resolveResult.error
+            );
             return resolveResult;
+          }
           const [depsValid, depsInvalid] = resolveResult.value;
 
           // add depsValid to pkgsInScope.
@@ -253,7 +263,11 @@ export function makeAddCmd(
           // print suggestion for depsInvalid
           let isAnyDependencyUnresolved = false;
           depsInvalid.forEach((depObj) => {
-            logPackumentResolveError(log, depObj.name, depObj.reason);
+            notifyRemotePackumentVersionResolvingFailed(
+              log,
+              depObj.name,
+              depObj.reason
+            );
 
             // If the manifest already has the dependency than it does not
             // really matter that it was not resolved.
@@ -330,16 +344,16 @@ export function makeAddCmd(
     // load manifest
     const loadResult = await loadProjectManifest(env.cwd).promise;
     if (loadResult.isErr()) {
-      logManifestLoadError(log, loadResult.error);
-      return loadResult;
+      notifyManifestLoadFailed(log, loadResult.error);
+      return ResultCodes.Error;
     }
     let manifest = loadResult.value;
 
     // add
     let dirty = false;
     for (const pkg of pkgs) {
       const result = await tryAddToManifest(manifest, pkg);
-      if (result.isErr()) return result;
+      if (result.isErr()) return ResultCodes.Error;
 
       const [newManifest, manifestChanged] = result.value;
       if (manifestChanged) {
@@ -352,14 +366,14 @@ export function makeAddCmd(
     if (dirty) {
       const saveResult = await writeProjectManifest(env.cwd, manifest).promise;
       if (saveResult.isErr()) {
-        logManifestSaveError(log, saveResult.error);
-        return saveResult;
+        notifyManifestWriteFailed(log);
+        return ResultCodes.Error;
       }
 
       // print manifest notice
       log.notice("", "please open Unity project to apply changes");
     }
 
-    return Ok(undefined);
+    return ResultCodes.Ok;
   };
 }
diff --git a/src/cli/cmd-deps.ts b/src/cli/cmd-deps.ts
@@ -1,4 +1,4 @@
-import { EnvParseError, ParseEnvService } from "../services/parse-env";
+import { ParseEnvService } from "../services/parse-env";
 import { isPackageUrl } from "../domain/package-url";
 import {
   makePackageReference,
@@ -8,23 +8,26 @@ import {
 import { CmdOptions } from "./options";
 import { PackumentVersionResolveError } from "../packument-version-resolving";
 import { PackumentNotFoundError } from "../common-errors";
-import { Ok, Result } from "ts-results-es";
-import {
-  DependencyResolveError,
-  ResolveDependenciesService,
-} from "../services/dependency-resolving";
+import { ResolveDependenciesService } from "../services/dependency-resolving";
 import { Logger } from "npmlog";
 import { logValidDependency } from "./dependency-logging";
 import { VersionNotFoundError } from "../domain/packument";
-import { logEnvParseError } from "./error-logging";
 import { DebugLog } from "../logging";
-
-export type DepsError = EnvParseError | DependencyResolveError;
+import { ResultCodes } from "./result-codes";
+import {
+  notifyEnvParsingFailed,
+  notifyRemotePackumentVersionResolvingFailed,
+} from "./error-logging";
 
 export type DepsOptions = CmdOptions<{
   deep?: boolean;
 }>;
 
+/**
+ * The possible result codes with which the deps command can exit.
+ */
+export type DepsResultCode = ResultCodes.Ok | ResultCodes.Error;
+
 /**
  * Cmd-handler for listing dependencies for a package.
  * @param pkg Reference to a package.
@@ -33,7 +36,7 @@ export type DepsOptions = CmdOptions<{
 export type DepsCmd = (
   pkg: PackageReference,
   options: DepsOptions
-) => Promise<Result<void, DepsError>>;
+) => Promise<DepsResultCode>;
 
 function errorPrefixForError(error: PackumentVersionResolveError): string {
   if (error instanceof PackumentNotFoundError) return "missing dependency";
@@ -55,16 +58,17 @@ export function makeDepsCmd(
     // parse env
     const envResult = await parseEnv(options);
     if (envResult.isErr()) {
-      logEnvParseError(log, envResult.error);
-      return envResult;
+      notifyEnvParsingFailed(log, envResult.error);
+      return ResultCodes.Error;
     }
     const env = envResult.value;
 
     const [name, version] = splitPackageReference(pkg);
 
-    if (version !== undefined && isPackageUrl(version))
-      // TODO: Convert to result
-      throw new Error("Cannot get dependencies for url-version");
+    if (version !== undefined && isPackageUrl(version)) {
+      log.error("", "cannot get dependencies for url-version");
+      return ResultCodes.Error;
+    }
 
     const deep = options.deep || false;
     debugLog(`fetch: ${makePackageReference(name, version)}, deep=${deep}`);
@@ -74,10 +78,15 @@ export function makeDepsCmd(
       version,
       deep
     );
-    if (resolveResult.isErr())
-      // TODO: Log errors
-      // TODO: Add tests
-      return resolveResult;
+    if (resolveResult.isErr()) {
+      notifyRemotePackumentVersionResolvingFailed(
+        log,
+        name,
+        resolveResult.error
+      );
+      return ResultCodes.Error;
+    }
+
     const [depsValid, depsInvalid] = resolveResult.value;
 
     depsValid.forEach((dependency) => logValidDependency(debugLog, dependency));
@@ -93,6 +102,6 @@ export function makeDepsCmd(
         log.warn(prefix, x.name);
       });
 
-    return Ok(undefined);
+    return ResultCodes.Ok;
   };
 }
diff --git a/src/cli/cmd-login.ts b/src/cli/cmd-login.ts
@@ -1,6 +1,5 @@
-import { AuthenticationError } from "../services/npm-login";
-import { GetUpmConfigPath, GetUpmConfigPathError } from "../io/upm-config-io";
-import { EnvParseError, ParseEnvService } from "../services/parse-env";
+import { GetUpmConfigPath } from "../io/upm-config-io";
+import { ParseEnvService } from "../services/parse-env";
 import { coerceRegistryUrl } from "../domain/registry-url";
 import {
   promptEmail,
@@ -9,23 +8,11 @@ import {
   promptUsername,
 } from "./prompts";
 import { CmdOptions } from "./options";
-import { Ok, Result } from "ts-results-es";
-import { NpmrcLoadError, NpmrcSaveError } from "../io/npmrc-io";
 import { Logger } from "npmlog";
-import { UpmAuthStoreError } from "../services/upm-auth";
 import { LoginService } from "../services/login";
-import { logEnvParseError } from "./error-logging";
-
-/**
- * Errors which may occur when logging in.
- */
-export type LoginError =
-  | EnvParseError
-  | GetUpmConfigPathError
-  | AuthenticationError
-  | NpmrcLoadError
-  | NpmrcSaveError
-  | UpmAuthStoreError;
+import { ResultCodes } from "./result-codes";
+import { RegistryAuthenticationError } from "../io/common-errors";
+import { notifyEnvParsingFailed } from "./error-logging";
 
 /**
  * Options for logging in a user. These come from the CLI.
@@ -40,13 +27,16 @@ export type LoginOptions = CmdOptions<{
   alwaysAuth?: boolean;
 }>;
 
+/**
+ * The possible result codes with which the login command can exit.
+ */
+export type LoginResultCode = ResultCodes.Ok | ResultCodes.Error;
+
 /**
  * Cmd-handler for logging in users.
  * @param options Options for logging in.
  */
-export type LoginCmd = (
-  options: LoginOptions
-) => Promise<Result<void, LoginError>>;
+export type LoginCmd = (options: LoginOptions) => Promise<LoginResultCode>;
 
 /**
  * Makes a {@link LoginCmd} function.
@@ -61,8 +51,8 @@ export function makeLoginCmd(
     // parse env
     const envResult = await parseEnv(options);
     if (envResult.isErr()) {
-      logEnvParseError(log, envResult.error);
-      return envResult;
+      notifyEnvParsingFailed(log, envResult.error);
+      return ResultCodes.Error;
     }
     const env = envResult.value;
 
@@ -80,7 +70,10 @@ export function makeLoginCmd(
 
     const configPathResult = await getUpmConfigPath(env.wsl, env.systemUser)
       .promise;
-    if (configPathResult.isErr()) return configPathResult;
+    if (configPathResult.isErr()) {
+      // TODO: Log error
+      return ResultCodes.Error;
+    }
     const configPath = configPathResult.value;
 
     const loginResult = await login(
@@ -95,17 +88,15 @@ export function makeLoginCmd(
 
     if (loginResult.isErr()) {
       const loginError = loginResult.error;
-      if (loginError instanceof AuthenticationError) {
-        if (loginError.status === 401)
-          log.warn("401", "Incorrect username or password");
-        else log.error(loginError.status.toString(), loginError.message);
-      }
+      if (loginError instanceof RegistryAuthenticationError)
+        log.warn("401", "Incorrect username or password");
 
-      return loginResult;
+      // TODO: Log all errors
+      return ResultCodes.Error;
     }
 
     log.notice("auth", `you are authenticated as '${username}'`);
     log.notice("config", "saved unity config at " + configPath);
-    return Ok(undefined);
+    return ResultCodes.Ok;
   };
 }