Skip to content

Commit

Permalink
Avoid console.error in CLI (#27657)
Browse files Browse the repository at this point in the history
Recent versions of Node.js [print console.error() in red](nodejs/node#51629), changing the color of convex CLI output unintentionally. Change everywhere we `console.error` as a means to write to stderr to `process.stderr.write` calls.

GitOrigin-RevId: 134e5302858c309e3a7fd9d470a685d9bf60b1df
  • Loading branch information
thomasballinger authored and Convex, Inc. committed Jul 13, 2024
1 parent ecfc769 commit 5895dea
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 21 deletions.
18 changes: 12 additions & 6 deletions src/bundler/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as Sentry from "@sentry/node";
import chalk from "chalk";
import ora, { Ora } from "ora";
import { Filesystem, nodeFs } from "./fs.js";
import { format } from "util";

// How the error should be handled when running `npx convex dev`.
export type ErrorType =
Expand Down Expand Up @@ -56,22 +57,27 @@ async function flushAndExit(exitCode: number, err?: any) {
return process.exit(exitCode);
}

// console.error before it started being red by default in Node v20
function logToStderr(...args: unknown[]) {
process.stderr.write(`${format(...args)}\n`);
}

// Handles clearing spinner so that it doesn't get messed up
export function logError(ctx: Context, message: string) {
ctx.spinner?.clear();
console.error(message);
logToStderr(message);
}

// Handles clearing spinner so that it doesn't get messed up
export function logWarning(ctx: Context, message: string) {
ctx.spinner?.clear();
console.error(message);
logToStderr(message);
}

// Handles clearing spinner so that it doesn't get messed up
export function logMessage(ctx: Context, ...logged: any) {
ctx.spinner?.clear();
console.error(...logged);
logToStderr(...logged);
}

// For the rare case writing output to stdout. Status and error messages
Expand Down Expand Up @@ -105,7 +111,7 @@ export function changeSpinner(ctx: Context, message: string) {
// Add newline to prevent clobbering
ctx.spinner.text = message + "\n";
} else {
console.error(message);
logToStderr(message);
}
}

Expand All @@ -114,7 +120,7 @@ export function logFailure(ctx: Context, message: string) {
ctx.spinner.fail(message);
ctx.spinner = undefined;
} else {
console.error(`${chalk.red(`✖`)} ${message}`);
logToStderr(`${chalk.red(`✖`)} ${message}`);
}
}

Expand All @@ -124,7 +130,7 @@ export function logFinishedStep(ctx: Context, message: string) {
ctx.spinner.succeed(message);
ctx.spinner = undefined;
} else {
console.error(`${chalk.green(`✔`)} ${message}`);
logToStderr(`${chalk.green(`✔`)} ${message}`);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/bundler/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,23 +115,23 @@ test("returns true when multiple imports and httpRouter is imported", async () =

test("bundle warns about https.js|ts at top level", async () => {
const fixtureDir = dirname + "/test_fixtures/js/project_with_https";
const logSpy = vi.spyOn(console, "error");
const logSpy = vi.spyOn(process.stderr, "write");
await entryPoints(oneoffContext, fixtureDir, false);
expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("https"));
});

test("bundle does not warn about https.js|ts which is not at top level", async () => {
const fixtureDir =
dirname + "/test_fixtures/js/project_with_https_not_at_top_level";
const logSpy = vi.spyOn(console, "error");
const logSpy = vi.spyOn(process.stderr, "write");
await entryPoints(oneoffContext, fixtureDir, false);
expect(logSpy).toHaveBeenCalledTimes(0);
});

test("bundle does not warn about https.js|ts which does not import httpRouter", async () => {
const fixtureDir =
dirname + "/test_fixtures/js/project_with_https_without_router";
const logSpy = vi.spyOn(console, "error");
const logSpy = vi.spyOn(process.stderr, "write");
await entryPoints(oneoffContext, fixtureDir, false);
expect(logSpy).toHaveBeenCalledTimes(0);
});
Expand Down
15 changes: 10 additions & 5 deletions src/cli/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#!/usr/bin/env node
/* eslint-disable no-restricted-syntax */
import { Command } from "@commander-js/extra-typings";
import { init } from "./init.js";
Expand Down Expand Up @@ -29,10 +28,16 @@ import { env } from "./env.js";
import { data } from "./data.js";
import inquirer from "inquirer";
import inquirerSearchList from "inquirer-search-list";
import { format } from "util";

const MINIMUM_MAJOR_VERSION = 16;
const MINIMUM_MINOR_VERSION = 15;

// console.error before it started being red by default in Node.js v20
function logToStderr(...args: unknown[]) {
process.stderr.write(`${format(...args)}\n`);
}

async function main() {
// If you want to use `@sentry/tracing` in your project directly, use a named import instead:
// import * as SentryTracing from "@sentry/tracing"
Expand Down Expand Up @@ -63,25 +68,25 @@ async function main() {
(majorVersion === MINIMUM_MAJOR_VERSION &&
minorVersion < MINIMUM_MINOR_VERSION)
) {
console.error(
logToStderr(
chalk.red(
`Your Node version ${nodeVersion} is too old. Convex requires at least Node v${MINIMUM_MAJOR_VERSION}.${MINIMUM_MINOR_VERSION}`,
),
);
console.error(
logToStderr(
chalk.gray(
`You can use ${chalk.bold(
"nvm",
)} (https://github.com/nvm-sh/nvm#installing-and-updating) to manage different versions of Node.`,
),
);
console.error(
logToStderr(
chalk.gray(
"After installing `nvm`, install the latest version of Node with " +
chalk.bold("`nvm install node`."),
),
);
console.error(
logToStderr(
chalk.gray(
"Then, activate the installed version in your terminal with " +
chalk.bold("`nvm use`."),
Expand Down
13 changes: 6 additions & 7 deletions src/cli/lib/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,16 @@ test("parseProjectConfig", async () => {
throw new Error();
},
};
const consoleSpy = vi
.spyOn(global.console, "error")
.mockImplementation(() => {
// Do nothing
});
const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => {
// Do nothing
return true;
});
const assertParses = async (inp: any) => {
expect(await parseProjectConfig(ctx, inp)).toEqual(inp);
};
const assertParseError = async (inp: any, err: string) => {
await expect(parseProjectConfig(ctx, inp)).rejects.toThrow();
expect(consoleSpy).toHaveBeenCalledWith(err);
expect(stderrSpy).toHaveBeenCalledWith(err);
};

await assertParses({
Expand Down Expand Up @@ -60,6 +59,6 @@ test("parseProjectConfig", async () => {
functions: "functions/",
authInfo: [{}],
},
"Expected `authInfo` in `convex.json` to be of type AuthInfo[]",
"Expected `authInfo` in `convex.json` to be of type AuthInfo[]\n",
);
});

0 comments on commit 5895dea

Please sign in to comment.