Skip to content

Commit

Permalink
feat: basic env error logging (#341)
Browse files Browse the repository at this point in the history
* refactor: error property

Make error constructor parameter into property so it can be used in logging later.

* feat: basic env error logging

Add some basic error messages for when env parsing failed

* fix: duplicate dots
  • Loading branch information
ComradeVanti authored May 25, 2024
1 parent f1c8f8c commit eaca854
Show file tree
Hide file tree
Showing 14 changed files with 74 additions and 16 deletions.
6 changes: 5 additions & 1 deletion src/cli/cmd-add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { areArraysEqual } from "../utils/array-utils";
import { Err, Ok, Result } from "ts-results-es";
import { CustomError } from "ts-custom-error";
import {
logEnvParseError,
logManifestLoadError,
logManifestSaveError,
logPackumentResolveError,
Expand Down Expand Up @@ -117,7 +118,10 @@ export function makeAddCmd(
if (!Array.isArray(pkgs)) pkgs = [pkgs];
// parse env
const envResult = await parseEnv(options);
if (envResult.isErr()) return envResult;
if (envResult.isErr()) {
logEnvParseError(log, envResult.error);
return envResult;
}
const env = envResult.value;

if (typeof env.editorVersion === "string")
Expand Down
6 changes: 5 additions & 1 deletion src/cli/cmd-deps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import { Logger } from "npmlog";
import { logValidDependency } from "./dependency-logging";
import { VersionNotFoundError } from "../domain/packument";
import { logEnvParseError } from "./error-logging";

export type DepsError = EnvParseError | DependencyResolveError;

Expand Down Expand Up @@ -51,7 +52,10 @@ export function makeDepsCmd(
return async (pkg, options) => {
// parse env
const envResult = await parseEnv(options);
if (envResult.isErr()) return envResult;
if (envResult.isErr()) {
logEnvParseError(log, envResult.error);
return envResult;
}
const env = envResult.value;

const [name, version] = splitPackageReference(pkg);
Expand Down
6 changes: 5 additions & 1 deletion src/cli/cmd-login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ 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.
Expand Down Expand Up @@ -59,7 +60,10 @@ export function makeLoginCmd(
return async (options) => {
// parse env
const envResult = await parseEnv(options);
if (envResult.isErr()) return envResult;
if (envResult.isErr()) {
logEnvParseError(log, envResult.error);
return envResult;
}
const env = envResult.value;

// query parameters
Expand Down
11 changes: 9 additions & 2 deletions src/cli/cmd-remove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ import {
PackageWithVersionError,
PackumentNotFoundError,
} from "../common-errors";
import { logManifestLoadError, logManifestSaveError } from "./error-logging";
import {
logEnvParseError,
logManifestLoadError,
logManifestSaveError,
} from "./error-logging";
import { Logger } from "npmlog";

export type RemoveError =
Expand Down Expand Up @@ -59,7 +63,10 @@ export function makeRemoveCmd(
if (!Array.isArray(pkgs)) pkgs = [pkgs];
// parse env
const envResult = await parseEnv(options);
if (envResult.isErr()) return envResult;
if (envResult.isErr()) {
logEnvParseError(log, envResult.error);
return envResult;
}
const env = envResult.value;

const tryRemoveFromManifest = async function (
Expand Down
6 changes: 5 additions & 1 deletion src/cli/cmd-search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Ok, Result } from "ts-results-es";
import { HttpErrorBase } from "npm-registry-fetch/lib/errors";
import { Logger } from "npmlog";
import { SearchPackages } from "../services/search-packages";
import { logEnvParseError } from "./error-logging";

export type SearchError = EnvParseError | HttpErrorBase;

Expand All @@ -32,7 +33,10 @@ export function makeSearchCmd(
return async (keyword, options) => {
// parse env
const envResult = await parseEnv(options);
if (envResult.isErr()) return envResult;
if (envResult.isErr()) {
logEnvParseError(log, envResult.error);
return envResult;
}
const env = envResult.value;

let usedEndpoint = "npmsearch";
Expand Down
6 changes: 5 additions & 1 deletion src/cli/cmd-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
} from "../common-errors";
import { Logger } from "npmlog";
import { ResolveRemotePackument } from "../services/resolve-remote-packument";
import { logEnvParseError } from "./error-logging";

export type ViewOptions = CmdOptions;

Expand Down Expand Up @@ -111,7 +112,10 @@ export function makeViewCmd(
return async (pkg, options) => {
// parse env
const envResult = await parseEnv(options);
if (envResult.isErr()) return envResult;
if (envResult.isErr()) {
logEnvParseError(log, envResult.error);
return envResult;
}
const env = envResult.value;

// parse name
Expand Down
31 changes: 31 additions & 0 deletions src/cli/error-logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import { FileParseError, PackumentNotFoundError } from "../common-errors";
import { DomainName } from "../domain/domain-name";
import { VersionNotFoundError } from "../domain/packument";
import { FsError, FsErrorReason } from "../io/file-io";
import { EnvParseError } from "../services/parse-env";
import { NoWslError } from "../io/wsl";
import { ChildProcessError } from "../utils/process";
import { RequiredEnvMissingError } from "../io/upm-config-io";

/**
* Logs a {@link ManifestLoadError} to the console.
Expand Down Expand Up @@ -49,3 +53,30 @@ export function logPackumentResolveError(
);
}
}

/**
* Logs a {@link EnvParseError} to a logger.
*/
export function logEnvParseError(log: Logger, error: EnvParseError) {
// TODO: Formulate more specific error messages.
const reason =
error instanceof NoWslError
? "you attempted to use wsl even though you are not running openupm inside wsl"
: error instanceof FsError
? error.reason === FsErrorReason.Missing
? `the file or directory at "${error.path}" does not exist`
: `the file or directory at "${error.path}" could not be read`
: error instanceof ChildProcessError
? "a required child process failed"
: error instanceof FileParseError
? `the file at ${error.path} could not parsed into a ${error.targetDescription}`
: error instanceof RequiredEnvMissingError
? `none of the following environment variables were set: ${error.keyNames.join(
", "
)}`
: `a string was malformed. Expected to be ${error.formatName}`;
const errorMessage = `environment information could not be parsed because ${reason}.`;
log.error("", errorMessage);

// TODO: Suggest actions user might take in order to fix the problem.
}
2 changes: 1 addition & 1 deletion src/common-errors.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { CustomError } from "ts-custom-error";
import { EditorVersion } from "./domain/editor-version";
import {StringFormatError} from "./utils/string-parsing";
import { StringFormatError } from "./utils/string-parsing";

/**
* Error for when the packument was not found.
Expand Down
2 changes: 1 addition & 1 deletion src/io/special-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export function makeHomePathGetter(): GetHomePath {
return () => {
const homePath = tryGetEnv("USERPROFILE") ?? tryGetEnv("HOME");
if (homePath === null)
return Err(new RequiredEnvMissingError("USERPROFILE", "HOME"));
return Err(new RequiredEnvMissingError(["USERPROFILE", "HOME"]));
return Ok(homePath);
};
}
Expand Down
4 changes: 2 additions & 2 deletions src/io/upm-config-io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const configFileName = ".upmconfig.toml";

export class RequiredEnvMissingError extends CustomError {
private readonly _class = "RequiredEnvMissingError";
constructor(...keyNames: string[]) {
constructor(public readonly keyNames: string[]) {
super(
`Env was required to contain a value for one of the following keys, but all were missing: ${keyNames
.map((keyName) => `"${keyName}"`)
Expand Down Expand Up @@ -63,7 +63,7 @@ export function makeUpmConfigPathGetter(
const profilePath = tryGetEnv("ALLUSERSPROFILE");
if (profilePath === null)
return Err(
new RequiredEnvMissingError("ALLUSERSPROFILE")
new RequiredEnvMissingError(["ALLUSERSPROFILE"])
).toAsyncResult();
return Ok(path.join(profilePath, systemUserSubPath)).toAsyncResult();
}
Expand Down
4 changes: 2 additions & 2 deletions test/cli/cmd-login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe("cmd-login", () => {
// TODO: Add tests for prompting logic

it("should fail if upm config path could not be determined", async () => {
const expected = new RequiredEnvMissingError();
const expected = new RequiredEnvMissingError([]);
const { loginCmd, getUpmConfigPath } = makeDependencies();
getUpmConfigPath.mockReturnValue(Err(expected).toAsyncResult());

Expand All @@ -71,7 +71,7 @@ describe("cmd-login", () => {
});

it("should fail if login failed", async () => {
const expected = new RequiredEnvMissingError();
const expected = new RequiredEnvMissingError([]);
const { loginCmd, login } = makeDependencies();
login.mockReturnValue(Err(expected).toAsyncResult());

Expand Down
2 changes: 1 addition & 1 deletion test/io/npmrc-io.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe("npmrc-io", () => {

it("should fail if home could not be determined", () => {
const { findPath, getHomePath } = makeDependencies();
getHomePath.mockReturnValue(Err(new RequiredEnvMissingError()));
getHomePath.mockReturnValue(Err(new RequiredEnvMissingError([])));

const result = findPath();

Expand Down
2 changes: 1 addition & 1 deletion test/io/upm-config-io.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe("upm-config-io", () => {

it("should fail if home could not be determined", async () => {
const { getUpmConfigPath, getHomePath } = makeDependencies();
const expected = new RequiredEnvMissingError();
const expected = new RequiredEnvMissingError([]);
getHomePath.mockReturnValue(Err(expected));

const result = await getUpmConfigPath(false, false).promise;
Expand Down
2 changes: 1 addition & 1 deletion test/services/npmrc-auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function makeDependencies() {
describe("npmrc-auth", () => {
describe("set token", () => {
it("should fail if path could not be determined", async () => {
const expected = new RequiredEnvMissingError();
const expected = new RequiredEnvMissingError([]);
const { authNpmrc, findPath } = makeDependencies();
findPath.mockReturnValue(Err(expected));

Expand Down

0 comments on commit eaca854

Please sign in to comment.