Replace console.foo with helper functions

.eslintrc.js

@@ -44,15 +44,16 @@ module.exports = {
       { checkIntersections: false },
     ],
     "@typescript-eslint/unbound-method": ["error", { ignoreStatic: true }],
-    // Allow console.log() statements
-    "no-console": "off",
+    // Use `print1` and `print2` instead
+    "no-console": "error",
   },
   ignorePatterns: [
     ".eslintrc.js",
     "prettier.config.js",
     "jest.config.js",
     "public/**",
     "build/**",
+    "__mocks__/**",
     "node_modules/**",
   ],
   overrides: [],

src/commands/__tests__/__snapshots__/ls.test.ts.snap

@@ -19,18 +19,18 @@ Unknown argument: foo",
 exports[`ls when valid ls command should print list response 1`] = `
 [
   [
-    "Showing destinations:",
+    "instance-1",
+  ],
+  [
+    "instance-2",
   ],
 ]
 `;
 
 exports[`ls when valid ls command should print list response 2`] = `
 [
   [
-    "instance-1",
-  ],
-  [
-    "instance-2",
+    "Showing destinations:",
   ],
 ]
 `;

src/commands/__tests__/ls.test.ts

@@ -1,13 +1,15 @@
 import { fetchCommand } from "../../drivers/api";
+import { print1, print2 } from "../../drivers/stdio";
 import { lsCommand } from "../ls";
 import yargs from "yargs";
 
 jest.mock("../../drivers/api");
 jest.mock("../../drivers/auth");
+jest.mock("../../drivers/stdio");
 
 const mockFetchCommand = fetchCommand as jest.Mock;
-const stdout = jest.spyOn(global.console, "log");
-const stderr = jest.spyOn(global.console, "error");
+const mockPrint1 = print1 as jest.Mock;
+const mockPrint2 = print2 as jest.Mock;
 
 describe("ls", () => {
   describe("when valid ls command", () => {
@@ -24,8 +26,8 @@ describe("ls", () => {
 
     it("should print list response", async () => {
       await lsCommand(yargs).parse(command);
-      expect(stderr.mock.calls).toMatchSnapshot();
-      expect(stdout.mock.calls).toMatchSnapshot();
+      expect(mockPrint1.mock.calls).toMatchSnapshot();
+      expect(mockPrint2.mock.calls).toMatchSnapshot();
     });
   });
 

src/commands/__tests__/request.test.ts

@@ -1,4 +1,5 @@
 import { fetchCommand } from "../../drivers/api";
+import { print1, print2 } from "../../drivers/stdio";
 import { RequestResponse } from "../../types/request";
 import { sleep } from "../../util";
 import { requestCommand } from "../request";
@@ -7,10 +8,11 @@ import yargs from "yargs";
 
 jest.mock("../../drivers/api");
 jest.mock("../../drivers/auth");
+jest.mock("../../drivers/stdio");
 
 const mockFetchCommand = fetchCommand as jest.Mock;
-const stdout = jest.spyOn(global.console, "log");
-const stderr = jest.spyOn(global.console, "error");
+const mockPrint1 = print1 as jest.Mock;
+const mockPrint2 = print2 as jest.Mock;
 
 describe("request", () => {
   beforeEach(() => jest.clearAllMocks());
@@ -39,8 +41,8 @@ describe("request", () => {
         it(`should${should ? "" : " not"} print request response`, async () => {
           mockFetch({ isPreexisting, isPersistent });
           await requestCommand(yargs).parse(command);
-          expect(stderr.mock.calls).toMatchSnapshot();
-          expect(stdout).not.toHaveBeenCalled();
+          expect(mockPrint2.mock.calls).toMatchSnapshot();
+          expect(mockPrint1).not.toHaveBeenCalled();
         });
       }
     );
@@ -54,8 +56,8 @@ describe("request", () => {
         status: "DONE",
       });
       await expect(promise).resolves.toBeDefined();
-      expect(stderr.mock.calls).toMatchSnapshot();
-      expect(stdout).not.toHaveBeenCalled();
+      expect(mockPrint2.mock.calls).toMatchSnapshot();
+      expect(mockPrint1).not.toHaveBeenCalled();
     });
   });
 

src/commands/aws/role.ts

@@ -1,6 +1,7 @@
 import { parseXml } from "../../common/xml";
 import { authenticate } from "../../drivers/auth";
 import { guard } from "../../drivers/firestore";
+import { print1, print2 } from "../../drivers/stdio";
 import { getAwsConfig } from "../../plugins/aws/config";
 import { AwsItemConfig, AwsOktaSamlUidLocation } from "../../plugins/aws/types";
 import { assumeRoleWithOktaSaml } from "../../plugins/okta/aws";
@@ -75,15 +76,15 @@ const oktaAwsAssumeRole = async (args: { account?: string; role: string }) => {
   const authn = await authenticate();
   const awsCredential = await assumeRoleWithOktaSaml(authn, args);
   const isTty = sys.writeOutputIsTTY?.();
-  if (isTty) console.error("Execute the following commands:\n");
+  if (isTty) print2("Execute the following commands:\n");
   const indent = isTty ? "  " : "";
-  console.log(
+  print1(
     Object.entries(awsCredential)
       .map(([key, value]) => `${indent}export ${key}=${value}`)
       .join("\n")
   );
   if (isTty)
-    console.error(`
+    print2(`
 Or, populate these environment variables using BASH command substitution:
 
   $(p0 aws${args.account ? ` --account ${args.account}` : ""} role assume ${
@@ -115,13 +116,13 @@ const oktaAwsListRoles = async (args: { account?: string }) => {
     .filter((r) => r.startsWith(`arn:aws:iam::${account}:role/`))
     .map((r) => r.split("/").slice(1).join("/"));
   const isTty = sys.writeOutputIsTTY?.();
-  if (isTty) console.error(`Your available roles for account ${account}:`);
+  if (isTty) print2(`Your available roles for account ${account}:`);
   if (!roles?.length) {
     const accounts = uniq(arns.map((a) => a.split(":")[4])).sort();
     throw `No roles found. You have roles on these accounts:\n${accounts.join(
       "\n"
     )}`;
   }
   const indent = isTty ? "  " : "";
-  console.log(roles.map((r) => `${indent}${r}`).join("\n"));
+  print1(roles.map((r) => `${indent}${r}`).join("\n"));
 };

src/commands/index.ts

@@ -1,3 +1,4 @@
+import { print2 } from "../drivers/stdio";
 import { awsCommand } from "./aws";
 import { loginCommand } from "./login";
 import { lsCommand } from "./ls";
@@ -22,10 +23,10 @@ export const cli = commands
   .version(VERSION)
   .demandCommand(1)
   .fail((message, error, yargs) => {
-    if (error) console.error(error);
+    if (error) print2(error);
     else {
-      console.error(yargs.help());
-      console.error(`\n${message}`);
+      print2(yargs.help());
+      print2(`\n${message}`);
     }
     sys.exit(1);
   });

src/commands/login.ts

@@ -1,5 +1,6 @@
 import { IDENTITY_FILE_PATH, authenticate } from "../drivers/auth";
 import { doc, guard } from "../drivers/firestore";
+import { print2 } from "../drivers/stdio";
 import { googleLogin } from "../plugins/google/login";
 import { oktaLogin } from "../plugins/okta/login";
 import { TokenResponse } from "../types/oidc";
@@ -39,12 +40,12 @@ export const login = async (
   // validate auth
   if (!options?.skipAuthenticate) await authenticate({ noRefresh: true });
 
-  console.error(`You are now logged in, and can use the p0 CLI.`);
+  print2(`You are now logged in, and can use the p0 CLI.`);
 };
 
 const writeIdentity = async (org: OrgData, credential: TokenResponse) => {
   const expires_at = Date.now() * 1e-3 + credential.expires_in - 1; // Add 1 second safety margin
-  console.error(`Saving authorization to ${IDENTITY_FILE_PATH}.`);
+  print2(`Saving authorization to ${IDENTITY_FILE_PATH}.`);
   const dir = path.dirname(IDENTITY_FILE_PATH);
   await fs.mkdir(dir, { recursive: true });
   await fs.writeFile(

src/commands/ls.ts

@@ -1,6 +1,7 @@
 import { fetchCommand } from "../drivers/api";
 import { authenticate } from "../drivers/auth";
 import { guard } from "../drivers/firestore";
+import { print2, print1 } from "../drivers/stdio";
 import pluralize from "pluralize";
 import yargs from "yargs";
 
@@ -41,13 +42,13 @@ const ls = async (
   ]);
 
   if (data && "ok" in data && data.ok) {
-    console.error(
+    print2(
       `Showing${
         data.isTruncated ? ` the first ${data.items.length}` : ""
       } ${pluralize(data.arg)}${data.term ? ` matching '${data.term}'` : ""}:`
     );
     for (const item of data.items) {
-      console.log(item);
+      print1(item);
     }
   } else {
     throw data;

src/commands/request.ts

@@ -1,6 +1,7 @@
 import { fetchCommand } from "../drivers/api";
 import { authenticate } from "../drivers/auth";
 import { doc, guard } from "../drivers/firestore";
+import { print2 } from "../drivers/stdio";
 import { Authn } from "../types/identity";
 import { Request, RequestResponse } from "../types/request";
 import { onSnapshot } from "firebase/firestore";
@@ -56,9 +57,7 @@ const waitForRequest = async (
 ) =>
   await new Promise<number>((resolve) => {
     if (logMessage)
-      console.error(
-        "Will wait up to 5 minutes for this request to complete..."
-      );
+      print2("Will wait up to 5 minutes for this request to complete...");
     let cancel: NodeJS.Timeout | undefined = undefined;
     const unsubscribe = onSnapshot<Request, object>(
       doc(`o/${tenantId}/permission-requests/${requestId}`),
@@ -70,14 +69,14 @@ const waitForRequest = async (
           if (cancel) clearTimeout(cancel);
           unsubscribe?.();
           const { message, code } = COMPLETED_REQUEST_STATUSES[status];
-          if (code !== 0 || logMessage) console.error(message);
+          if (code !== 0 || logMessage) print2(message);
           resolve(code);
         }
       }
     );
     cancel = setTimeout(() => {
       unsubscribe?.();
-      console.error("Your request did not complete within 5 minutes.");
+      print2("Your request did not complete within 5 minutes.");
       resolve(4);
     }, WAIT_TIMEOUT);
   });
@@ -106,7 +105,7 @@ export const request = async (
       (options?.message === "approval-required" &&
         !data.isPreexisting &&
         !data.isPersistent);
-    if (logMessage) console.error(data.message);
+    if (logMessage) print2(data.message);
     const { id } = data;
     if (args.wait && id && userCredential.user.tenantId) {
       const code = await waitForRequest(

src/commands/ssh.ts

@@ -1,5 +1,6 @@
 import { authenticate } from "../drivers/auth";
 import { doc, guard } from "../drivers/firestore";
+import { print2 } from "../drivers/stdio";
 import { ssm } from "../plugins/aws/ssm";
 import { AwsSsh } from "../plugins/aws/types";
 import { Authn } from "../types/identity";
@@ -91,11 +92,11 @@ const ssh = async (args: yargs.ArgumentsCamelCase<{ instance: string }>) => {
     { message: "approval-required" }
   );
   if (!response) {
-    console.error("Did not receive access ID from server");
+    print2("Did not receive access ID from server");
     return;
   }
   const { id, isPreexisting } = response;
-  if (!isPreexisting) console.error("Waiting for access to be provisioned");
+  if (!isPreexisting) print2("Waiting for access to be provisioned");
   const requestData = await waitForProvisioning<AwsSsh>(authn, id);
   await ssm(authn, { ...requestData, id });
 };

src/drivers/auth.ts

@@ -1,6 +1,7 @@
 import { login } from "../commands/login";
 import { Authn, Identity } from "../types/identity";
 import { auth } from "./firestore";
+import { print2 } from "./stdio";
 import {
   OAuthProvider,
   SignInMethod,
@@ -45,7 +46,7 @@ export const cached = async <T>(
     return JSON.parse(data.toString("utf-8")) as T;
   } catch (error: any) {
     if (error?.code !== "ENOENT")
-      console.error(
+      print2(
         `Could not load credentials "${name}" from cache: ${error.message ?? error}`
       );
     return await loadCache();
@@ -63,7 +64,7 @@ export const loadCredentials = async (options?: {
       identity.credential.expires_at < Date.now() * 1e-3
     ) {
       await login({ org: identity.org.slug }, { skipAuthenticate: true });
-      console.error("\u200B"); // Force a new line
+      print2("\u200B"); // Force a new line
       return loadCredentials({ noRefresh: true });
     }
     return identity;

src/drivers/stdio.ts

@@ -0,0 +1,25 @@
+/** Functions to handle stdio
+ *
+ * These are essentially wrappers around console.foo, but allow for
+ * - Better testing
+ * - Later redirection / duplication
+ */
+
+/** Used to output machine-readable text to stdout
+ *
+ * In general this should not be used for text meant to be consumed
+ * only by humans.
+ */
+export function print1(message: any) {
+  // eslint-disable-next-line no-console
+  console.log(message);
+}
+
+/** Output human-consumable text to stderr
+ *
+ * In general this should not be used for machine-consumed text.
+ */
+export function print2(message: any) {
+  // eslint-disable-next-line no-console
+  console.error(message);
+}

src/index.ts

@@ -2,7 +2,7 @@ import { cli } from "./commands";
 import { noop } from "lodash";
 
 export const main = () => {
-  // We can suppress output here, as .fail() already prints errors
+  // We can suppress output here, as .fail() already print1 errors
   void (cli.parse() as any).catch(noop);
 };