Skip to content

Commit

Permalink
feat: improve cli output when building project [fixes DXJ-406] (#317)
Browse files Browse the repository at this point in the history
* feat: improve cli output when building project [fixes DXJ-406]

* fix review

* fix review

* remove --sk replacement

* add variable for exit code message

* remove additional instanceof check

* use instance of for errors

* restore chmod
  • Loading branch information
shamsartem authored Jul 4, 2023
1 parent 8decf46 commit 1da1fb0
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 68 deletions.
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@
"unused-imports"
],
"rules": {
"no-console": [
"error"
],
"arrow-body-style": [
"error",
"always"
Expand Down
6 changes: 2 additions & 4 deletions src/beforeBuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ const compileInstallationSpellAqua = async (tracing = false) => {
);
};

(async () => {
void (async () => {
await rm(VERSIONS_DIR_PATH, { recursive: true, force: true });
await mkdir(VERSIONS_DIR_PATH, { recursive: true });
await cp("package.json", join(VERSIONS_DIR_PATH, "cli.package.json"));
Expand All @@ -151,6 +151,4 @@ const compileInstallationSpellAqua = async (tracing = false) => {

await compileInstallationSpellAqua();
await compileInstallationSpellAqua(true);
})().catch((e) => {
return console.error(e);
});
})();
3 changes: 3 additions & 0 deletions src/commands/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ export default class Run extends BaseCommand<typeof Run> {
typeof result === "string" ? result : jsonStringify(result);

if (stringResult !== undefined) {
// If `--quite` flag is used then commandObj.log does nothing
// So we use console.log here instead
// eslint-disable-next-line no-console
console.log(stringResult);
}

Expand Down
3 changes: 3 additions & 0 deletions src/countlyInterceptor.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ const ERROR_HANDLED_BY_OCLIF_KEY = "errorHandledByOclif";
*/
export const createErrorPromise = (error) => {
if (error instanceof CLIError) {
// eslint-disable-next-line no-console
console.error(`Error: ${error.message}`);
} else {
// eslint-disable-next-line no-console
console.error(error);
}

Expand All @@ -81,6 +83,7 @@ export const createErrorPromise = (error) => {

return new Promise(() => {
setTimeout(() => {
// eslint-disable-next-line no-console
console.log(
"\nWasn't able to report this crash to Fluence Team. Please report it manually to https://github.com/fluencelabs/fluence-cli/issues"
);
Expand Down
2 changes: 1 addition & 1 deletion src/genConfigDocs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,4 @@ ${schemas
);
};

main().catch(console.error);
void main();
79 changes: 42 additions & 37 deletions src/lib/execPromise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,20 @@
import { spawn } from "node:child_process";

import oclifColor from "@oclif/color";
import { CLIError } from "@oclif/core/lib/errors/index.js";

const color = oclifColor.default;
import { CLIError } from "@oclif/core/lib/errors/index.js";

import { IS_DEVELOPMENT, MARINE_CARGO_DEPENDENCY } from "./const.js";
import { isInteractive } from "./commandObj.js";
import { MARINE_CARGO_DEPENDENCY } from "./const.js";
import { type Flags, flagsToArgs } from "./helpers/flagsToArgs.js";
import { startSpinner, stopSpinner } from "./helpers/spinner.js";

type SpawnParams = Parameters<typeof spawn>;

const getErrorMessage = (printOutput: boolean, stderr: string) => {
return printOutput ? "" : `. Reason:\n${stderr}`;
};

export type ExecPromiseArg = {
command: SpawnParams[0];
args?: SpawnParams[1] | undefined;
Expand All @@ -37,33 +41,32 @@ export type ExecPromiseArg = {
printOutput?: boolean | undefined;
};

export const execPromise = ({
export const execPromise = async ({
command,
args,
flags,
options,
spinnerMessage,
timeout,
printOutput = false,
printOutput: printOutputArg = false,
}: ExecPromiseArg): Promise<string> => {
const printOutput = isInteractive ? printOutputArg : false;
const flagArgs = flags === undefined ? [] : flagsToArgs(flags);
const allArgs = [...(args ?? []), ...flagArgs];
const fullCommand = [command, ...allArgs].join(" ");

const commandToDisplay = IS_DEVELOPMENT
? fullCommand
: fullCommand.replace(
/([\S\s]*--sk ).*([\S\s]*)/,
"$1<SECRET_KEY_IS_HIDDEN>$2"
);
const getCommandFailedMessage = (code: number | null = null) => {
const exitCodeMessage =
code === null ? "failed" : `exited with code ${code}`;

return `Command: ${color.yellow(fullCommand)} ${exitCodeMessage}`;
};

if (typeof spinnerMessage === "string") {
startSpinner(spinnerMessage);
}

const debugInfo = `Debug info:\n${commandToDisplay}\n`;

return new Promise<string>((res, rej): void => {
const result = await new Promise<CLIError | Error | string>((res): void => {
const execTimeout =
timeout !== undefined &&
setTimeout((): void => {
Expand All @@ -72,12 +75,13 @@ export const execPromise = ({
}

childProcess.kill();
const commandFailedMessage = getCommandFailedMessage();

rej(
res(
new Error(
`Execution timed out: command didn't yield any result in ${color.yellow(
`${commandFailedMessage}. Reason: Execution timed out: command didn't yield any result in ${color.yellow(
`${timeout}ms`
)}\n${debugInfo}`
)}`
)
);
}, timeout);
Expand Down Expand Up @@ -111,7 +115,9 @@ export const execPromise = ({
stopSpinner(color.red("failed"));
}

rej(new Error(`${printOutput ? "" : stderr}${error}\n${debugInfo}`));
const commandFailedMessage = getCommandFailedMessage();
const errorMessage = getErrorMessage(printOutput, stderr);
res(new Error(`${commandFailedMessage}${errorMessage}${error}`));
});

childProcess.on("close", (code): void => {
Expand All @@ -126,32 +132,31 @@ export const execPromise = ({
}

if (stderr.includes("linker `cc` not found")) {
rej(
new CLIError(
`\n${color.yellow(MARINE_CARGO_DEPENDENCY)} requires ${color.yellow(
"build-essential"
)} to be installed. Please install it and try again.\nOn debian-based systems (e.g. Ubuntu) you can install it using this command:\n\n${color.yellow(
"sudo apt install build-essential"
)}\n`
)
);

const expectedErrorMessage = `\n${color.yellow(
MARINE_CARGO_DEPENDENCY
)} requires ${color.yellow(
"build-essential"
)} to be installed. Please install it and try again.\nOn debian-based systems (e.g. Ubuntu) you can install it using this command:\n\n${color.yellow(
"sudo apt install build-essential"
)}\n`;

res(new CLIError(expectedErrorMessage));
return;
}

if (code !== 0) {
rej(
new Error(
`${printOutput ? "" : stderr}\nprocess exited${
code === null ? "" : ` with code ${code}`
}\n${debugInfo}`
)
);

return;
const commandFailedMessage = getCommandFailedMessage(code);
const errorMessage = getErrorMessage(printOutput, stderr);
res(new Error(`${commandFailedMessage}${errorMessage}`));
}

res(stdout);
});
});

if (result instanceof Error) {
throw result;
}

return result;
};
1 change: 1 addition & 0 deletions src/lib/helpers/logAndFail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

export const logAndFail = (unknown: unknown): never => {
// eslint-disable-next-line no-console
console.log(unknown);
throw new Error("Failed after console.log");
};
31 changes: 21 additions & 10 deletions src/lib/marineCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@

import { join } from "node:path";

import { commandObj } from "./commandObj.js";
import type { FluenceConfig } from "./configs/project/fluence.js";
import { BIN_DIR_NAME, MARINE_CARGO_DEPENDENCY } from "./const.js";
import { execPromise } from "./execPromise.js";
import { getMessageWithKeyValuePairs } from "./helpers/getMessageWithKeyValuePairs.js";
import { ensureCargoDependency } from "./rust.js";
import type { Flags } from "./typeHelpers.js";
import { type Flags } from "./typeHelpers.js";

type MarineCliInput =
| {
Expand Down Expand Up @@ -62,16 +63,26 @@ export const initMarineCli = async (
cwd,
printOutput = true,
}): Promise<string> => {
return execPromise({
command: marineCLIPath,
args,
flags,
spinnerMessage:
try {
const spinnerMessage =
message === undefined
? undefined
: getMessageWithKeyValuePairs(message, keyValuePairs),
options: { cwd },
printOutput,
});
: getMessageWithKeyValuePairs(message, keyValuePairs);

return await execPromise({
command: marineCLIPath,
args,
flags,
spinnerMessage,
options: { cwd },
printOutput,
});
} catch (e) {
if (e instanceof Error) {
return commandObj.error(e.message);
}

throw e;
}
};
};
28 changes: 13 additions & 15 deletions src/lib/rust.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
import { addCountlyLog } from "./countly.js";
import { execPromise } from "./execPromise.js";
import { downloadFile } from "./helpers/downloadFile.js";
import { jsonStringify } from "./helpers/jsonStringify.js";
import {
handleInstallation,
resolveDependencies,
Expand All @@ -43,7 +44,6 @@ import {
} from "./helpers/package.js";
import { replaceHomeDir } from "./helpers/replaceHomeDir.js";
import { startSpinner, stopSpinner } from "./helpers/spinner.js";
import { hasKey, isObject } from "./typeHelpers.js";

const CARGO = "cargo";
const RUSTUP = "rustup";
Expand Down Expand Up @@ -285,10 +285,19 @@ const tryDownloadingBinary = async ({

const url = `https://github.com/fluencelabs/marine/releases/download/${name}-v${version}/${name}-${platformToUse}-x86_64`;

startSpinner(
`Downloading ${name}@${version} binary to ${replaceHomeDir(
dependencyDirPath
)}`
);

try {
await downloadFile(binaryPath, url);
} catch {
return `Failed to download ${name}@${version} from ${url}`;
stopSpinner();
} catch (e) {
stopSpinner("failed");
const error = e instanceof Error ? e.message : jsonStringify(e);
return `Failed to download ${name}@${version} from ${url}. Error: ${error}`;
}

try {
Expand All @@ -311,12 +320,7 @@ const tryDownloadingBinary = async ({
return `Downloaded ${name}@${version} binary at ${binaryPath} --help message does not contain the ${version} version it is supposed to contain:\n result of --help execution is: ${helpText}`;
}
} catch (e) {
if (
isObject(e) &&
hasKey("message", e) &&
typeof e["message"] === "string" &&
e["message"].includes(version)
) {
if (e instanceof Error && e.message.includes(version)) {
return true;
}

Expand Down Expand Up @@ -369,10 +373,6 @@ export const ensureCargoDependency = async ({
version,
});

startSpinner(
`Installing ${name}@${version} to ${replaceHomeDir(dependencyDirPath)}`
);

const maybeErrorMessage = await tryDownloadingBinary({
name,
version,
Expand Down Expand Up @@ -404,8 +404,6 @@ export const ensureCargoDependency = async ({
});
}

stopSpinner();

await updateConfigsIfVersionChanged({
maybeFluenceConfig,
name,
Expand Down
2 changes: 1 addition & 1 deletion test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ export const initFirstTime = async (template: Template) => {
}
}

// eslint-disable-next-line no-console
console.log(`Initialized template "${template}"`);

return templatePath;
};

Expand Down
1 change: 1 addition & 0 deletions test/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe("integration tests", () => {
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
{
log(msg: string) {
// eslint-disable-next-line no-console
console.log(msg);
},
error(msg: string) {
Expand Down
2 changes: 2 additions & 0 deletions test/setupTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import "../src/lib/setupEnvironment.js";
import { fluence, initFirstTime } from "./helpers.js";

(async (): Promise<void> => {
// eslint-disable-next-line no-console
console.log("Setting up tests...");

await fluence({
Expand All @@ -32,6 +33,7 @@ import { fluence, initFirstTime } from "./helpers.js";
})
);

// eslint-disable-next-line no-console
console.log("Tests are ready to run!");
})().catch((error) => {
throw error;
Expand Down

0 comments on commit 1da1fb0

Please sign in to comment.