refactor: separate user and debug logs

src/cli/cmd-add.ts

@@ -53,6 +53,7 @@ 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";
 
 export class InvalidPackumentDataError extends CustomError {
   private readonly _class = "InvalidPackumentDataError";
@@ -112,7 +113,8 @@ export function makeAddCmd(
   resolveDependencies: ResolveDependenciesService,
   loadProjectManifest: LoadProjectManifest,
   writeProjectManifest: WriteProjectManifest,
-  log: Logger
+  log: Logger,
+  debugLog: DebugLog
 ): AddCmd {
   return async (pkgs, options) => {
     if (!Array.isArray(pkgs)) pkgs = [pkgs];
@@ -213,10 +215,7 @@ export function makeAddCmd(
 
         // pkgsInScope
         if (!isUpstreamPackage) {
-          log.verbose(
-            "dependency",
-            `fetch: ${makePackageReference(name, requestedVersion)}`
-          );
+          debugLog(`fetch: ${makePackageReference(name, requestedVersion)}`);
           const resolveResult = await resolveDependencies(
             [env.registry, env.upstreamRegistry],
             name,
@@ -231,7 +230,7 @@ export function makeAddCmd(
 
           // add depsValid to pkgsInScope.
           depsValid.forEach((dependency) =>
-            logValidDependency(log, dependency)
+            logValidDependency(debugLog, dependency)
           );
           depsValid
             .filter((x) => {

src/cli/cmd-deps.ts

@@ -17,6 +17,7 @@ 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;
 
@@ -47,7 +48,8 @@ function errorPrefixForError(error: PackumentVersionResolveError): string {
 export function makeDepsCmd(
   parseEnv: ParseEnvService,
   resolveDependencies: ResolveDependenciesService,
-  log: Logger
+  log: Logger,
+  debugLog: DebugLog
 ): DepsCmd {
   return async (pkg, options) => {
     // parse env
@@ -65,10 +67,7 @@ export function makeDepsCmd(
       throw new Error("Cannot get dependencies for url-version");
 
     const deep = options.deep || false;
-    log.verbose(
-      "dependency",
-      `fetch: ${makePackageReference(name, version)}, deep=${deep}`
-    );
+    debugLog(`fetch: ${makePackageReference(name, version)}, deep=${deep}`);
     const resolveResult = await resolveDependencies(
       [env.registry, env.upstreamRegistry],
       name,
@@ -81,7 +80,7 @@ export function makeDepsCmd(
       return resolveResult;
     const [depsValid, depsInvalid] = resolveResult.value;
 
-    depsValid.forEach((dependency) => logValidDependency(log, dependency));
+    depsValid.forEach((dependency) => logValidDependency(debugLog, dependency));
     depsValid
       .filter((x) => !x.self)
       .forEach((x) =>

src/cli/cmd-login.ts

@@ -90,9 +90,7 @@ export function makeLoginCmd(
       alwaysAuth,
       loginRegistry,
       configPath,
-      options.basicAuth ? "basic" : "token",
-      () => log.notice("auth", `you are authenticated as '${username}'`),
-      (npmrcPath) => log.notice("config", `saved to npm config: ${npmrcPath}`)
+      options.basicAuth ? "basic" : "token"
     ).promise;
 
     if (loginResult.isErr()) {
@@ -106,6 +104,7 @@ export function makeLoginCmd(
       return loginResult;
     }
 
+    log.notice("auth", `you are authenticated as '${username}'`);
     log.notice("config", "saved unity config at " + configPath);
     return Ok(undefined);
   };

src/cli/cmd-search.ts

@@ -7,6 +7,7 @@ import { HttpErrorBase } from "npm-registry-fetch/lib/errors";
 import { Logger } from "npmlog";
 import { SearchPackages } from "../services/search-packages";
 import { logEnvParseError } from "./error-logging";
+import { DebugLog } from "../logging";
 
 export type SearchError = EnvParseError | HttpErrorBase;
 
@@ -28,7 +29,8 @@ export type SearchCmd = (
 export function makeSearchCmd(
   parseEnv: ParseEnvService,
   searchPackages: SearchPackages,
-  log: Logger
+  log: Logger,
+  debugLog: DebugLog
 ): SearchCmd {
   return async (keyword, options) => {
     // parse env
@@ -56,7 +58,7 @@ export function makeSearchCmd(
       return Ok(undefined);
     }
 
-    log.verbose(usedEndpoint, results.map((it) => it.name).join(os.EOL));
+    debugLog(`${usedEndpoint}: ${results.map((it) => it.name).join(os.EOL)}`);
     console.log(formatAsTable(results));
     return Ok(undefined);
   };

src/cli/dependency-logging.ts

@@ -1,12 +1,15 @@
 import { ValidDependency } from "../services/dependency-resolving";
 import { makePackageReference } from "../domain/package-reference";
-import { Logger } from "npmlog";
 import { unityRegistryUrl } from "../domain/registry-url";
+import { DebugLog } from "../logging";
 
 /**
  * Logs information about a valid dependency to a logger.
  */
-export function logValidDependency(log: Logger, dependency: ValidDependency) {
+export function logValidDependency(
+  debugLog: DebugLog,
+  dependency: ValidDependency
+) {
   const packageRef = makePackageReference(dependency.name, dependency.version);
   const tag =
     dependency.source === "built-in"
@@ -15,5 +18,5 @@ export function logValidDependency(log: Logger, dependency: ValidDependency) {
       ? "[upstream]"
       : "";
   const message = `${packageRef} ${tag}`;
-  log.verbose("dependency", message);
+  debugLog(message);
 }

src/cli/index.ts

@@ -46,15 +46,23 @@ import { makePackumentFetcher } from "../io/packument-io";
 import { makePackagesSearcher } from "../services/search-packages";
 import { makeRemotePackumentResolver } from "../services/resolve-remote-packument";
 import { makeLoginService } from "../services/login";
+import { DebugLog } from "../logging";
 
 // Composition root
 
 const log = npmlog;
+const debugLog: DebugLog = (message, context) =>
+  log.verbose(
+    "openupm-cli",
+    `${message}${
+      context !== undefined ? ` context: ${JSON.stringify(context)}` : ""
+    }`
+  );
 const regClient = new RegClient({ log });
 const getCwd = makeCwdGetter();
 const getHomePath = makeHomePathGetter();
-const readFile = makeTextReader();
-const writeFile = makeTextWriter();
+const readFile = makeTextReader(debugLog);
+const writeFile = makeTextWriter(debugLog);
 const loadProjectManifest = makeProjectManifestLoader(readFile);
 const writeProjectManifest = makeProjectManifestWriter(writeFile);
 const getUpmConfigPath = makeUpmConfigPathGetter(getHomePath);
@@ -89,19 +97,25 @@ const saveAuthToUpmConfig = makeSaveAuthToUpmConfigService(
   writeFile
 );
 const searchPackages = makePackagesSearcher(searchRegistry, fetchAllPackuments);
-const login = makeLoginService(saveAuthToUpmConfig, npmLogin, authNpmrc);
+const login = makeLoginService(
+  saveAuthToUpmConfig,
+  npmLogin,
+  authNpmrc,
+  debugLog
+);
 
 const addCmd = makeAddCmd(
   parseEnv,
   resolveRemovePackumentVersion,
   resolveDependencies,
   loadProjectManifest,
   writeProjectManifest,
-  log
+  log,
+  debugLog
 );
 const loginCmd = makeLoginCmd(parseEnv, getUpmConfigPath, login, log);
-const searchCmd = makeSearchCmd(parseEnv, searchPackages, log);
-const depsCmd = makeDepsCmd(parseEnv, resolveDependencies, log);
+const searchCmd = makeSearchCmd(parseEnv, searchPackages, log, debugLog);
+const depsCmd = makeDepsCmd(parseEnv, resolveDependencies, log, debugLog);
 const removeCmd = makeRemoveCmd(
   parseEnv,
   loadProjectManifest,

src/io/builtin-packages.ts

@@ -8,6 +8,7 @@ import { DomainName } from "../domain/domain-name";
 import { CustomError } from "ts-custom-error";
 import path from "path";
 import { FsError, FsErrorReason, tryGetDirectoriesIn } from "./file-io";
+import { DebugLog } from "../logging";
 
 /**
  * Error for when an editor-version is not installed.
@@ -43,7 +44,9 @@ export type FindBuiltInPackages = (
 /**
  * Makes a {@link FindBuiltInPackages} function.
  */
-export function makeBuiltInPackagesFinder(): FindBuiltInPackages {
+export function makeBuiltInPackagesFinder(
+  debugLog: DebugLog
+): FindBuiltInPackages {
   return (editorVersion) => {
     {
       const pathResult = tryGetEditorInstallPath(editorVersion);
@@ -55,7 +58,7 @@ export function makeBuiltInPackagesFinder(): FindBuiltInPackages {
       );
 
       return (
-        tryGetDirectoriesIn(packagesDir)
+        tryGetDirectoriesIn(packagesDir, debugLog)
           // We can assume correct format
           .map((names) => names as DomainName[])
           .mapErr((error) =>

src/io/file-io.ts

@@ -4,6 +4,7 @@ import { CustomError } from "ts-custom-error";
 import { assertIsNodeError } from "../utils/error-type-guards";
 import fse from "fs-extra";
 import path from "path";
+import { DebugLog } from "../logging";
 
 /**
  * Reason why a file-system operation failed.
@@ -40,12 +41,13 @@ export class FsError extends CustomError {
   }
 }
 
-function fsOperation<T>(path: string, op: () => Promise<T>) {
+function fsOperation<T>(debugLog: DebugLog, op: () => Promise<T>) {
   return new AsyncResult(Result.wrapAsync(op)).mapErr((error) => {
     assertIsNodeError(error);
+    debugLog("fs-operation failed", error);
     const cause =
       error.code === "ENOENT" ? FsErrorReason.Missing : FsErrorReason.Other;
-    return new FsError(path, cause);
+    return new FsError(error.path!, cause);
   });
 }
 
@@ -64,9 +66,9 @@ export type ReadTextFile = (path: string) => AsyncResult<string, FileReadError>;
 /**
  * Makes a {@link ReadTextFile} function.
  */
-export function makeTextReader(): ReadTextFile {
+export function makeTextReader(debugLog: DebugLog): ReadTextFile {
   return (path) =>
-    fsOperation(path, () => fs.readFile(path, { encoding: "utf8" }));
+    fsOperation(debugLog, () => fs.readFile(path, { encoding: "utf8" }));
 }
 
 /**
@@ -88,11 +90,11 @@ export type WriteTextFile = (
 /**
  * Makes a {@link WriteTextFile} function.
  */
-export function makeTextWriter(): WriteTextFile {
+export function makeTextWriter(debugLog: DebugLog): WriteTextFile {
   return (filePath, content) => {
     const dirPath = path.dirname(filePath);
-    return fsOperation(dirPath, () => fse.ensureDir(dirPath)).andThen(() =>
-      fsOperation(filePath, () => fs.writeFile(filePath, content))
+    return fsOperation(debugLog, () => fse.ensureDir(dirPath)).andThen(() =>
+      fsOperation(debugLog, () => fs.writeFile(filePath, content))
     );
   };
 }
@@ -105,11 +107,14 @@ export type GetDirectoriesError = FsError;
 /**
  * Attempts to get the names of all directories in a directory.
  * @param directoryPath The directories name.
+ * @param debugLog Debug-log function.
+ * TODO: Convert to service function.
  */
 export function tryGetDirectoriesIn(
-  directoryPath: string
+  directoryPath: string,
+  debugLog: DebugLog
 ): AsyncResult<ReadonlyArray<string>, GetDirectoriesError> {
-  return fsOperation(directoryPath, () =>
+  return fsOperation(debugLog, () =>
     fs.readdir(directoryPath, { withFileTypes: true })
   )
     .map((entries) => entries.filter((it) => it.isDirectory()))

src/logging.ts

@@ -0,0 +1,16 @@
+/**
+ * Function that logs a message to some external system. This interface is
+ * output-agnostic meaning that the logs could go to a console, file or other.
+ * This interface is supposed to be only used for debug logs, not for
+ * user-facing logs.
+ * @param message The message to be logged.
+ * @param context Optional context object. Will be stringified and appended
+ * to the message.
+ * @returns Nothing. Could be asynchronous.
+ */
+export type DebugLog = (
+  message: string,
+  context?: object
+) => void | Promise<void>;
+
+export const noopLogger: DebugLog = () => {};

src/services/login.ts

@@ -4,6 +4,7 @@ import { RegistryUrl } from "../domain/registry-url";
 import { SaveAuthToUpmConfig, UpmAuthStoreError } from "./upm-auth";
 import { NpmLoginError, NpmLoginService } from "./npm-login";
 import { AuthNpmrcService, NpmrcAuthTokenUpdateError } from "./npmrc-auth";
+import { DebugLog } from "../logging";
 
 /**
  * Error which may occur when logging in a user.
@@ -37,9 +38,7 @@ export type LoginService = (
   alwaysAuth: boolean,
   registry: RegistryUrl,
   configPath: string,
-  authMode: "basic" | "token",
-  onNpmAuthSuccess: () => void,
-  onNpmrcUpdated: (npmrcPath: string) => void
+  authMode: "basic" | "token"
 ) => AsyncResult<void, LoginError>;
 
 /**
@@ -48,7 +47,8 @@ export type LoginService = (
 export function makeLoginService(
   saveAuthToUpmConfig: SaveAuthToUpmConfig,
   npmLogin: NpmLoginService,
-  authNpmrc: AuthNpmrcService
+  authNpmrc: AuthNpmrcService,
+  debugLog: DebugLog
 ): LoginService {
   return (
     username,
@@ -57,9 +57,7 @@ export function makeLoginService(
     alwaysAuth,
     registry,
     configPath,
-    authMode,
-    onNpmAuthSuccess,
-    onNpmrcUpdated
+    authMode
   ) => {
     if (authMode === "basic") {
       // basic auth
@@ -73,10 +71,10 @@ export function makeLoginService(
 
     // npm login
     return npmLogin(registry, username, password, email).andThen((token) => {
-      onNpmAuthSuccess();
+      debugLog(`npm login successful`);
       // write npm token
       return authNpmrc(registry, token).andThen((npmrcPath) => {
-        onNpmrcUpdated(npmrcPath);
+        debugLog(`saved to npm config: ${npmrcPath}`);
         // Save config
         return saveAuthToUpmConfig(configPath, registry, {
           email,