Skip to content

feat: improve error messages #347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 46 additions & 32 deletions src/cli/cmd-add.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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";
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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;

Expand Down Expand Up @@ -175,7 +177,11 @@ export function makeAddCmd(
}

if (resolveResult.isErr()) {
logPackumentResolveError(log, name, resolveResult.error);
notifyRemotePackumentVersionResolvingFailed(
log,
name,
resolveResult.error
);
return resolveResult;
}

Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
};
}
49 changes: 29 additions & 20 deletions src/cli/cmd-deps.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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.
Expand All @@ -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";
Expand All @@ -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}`);
Expand All @@ -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));
Expand All @@ -93,6 +102,6 @@ export function makeDepsCmd(
log.warn(prefix, x.name);
});

return Ok(undefined);
return ResultCodes.Ok;
};
}
53 changes: 22 additions & 31 deletions src/cli/cmd-login.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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;

Expand All @@ -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(
Expand All @@ -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;
};
}
Loading
Loading