feat: standardize CLI commands and errors (#930)

https://coveord.atlassian.net/browse/CDX-1138

packages/cli/commons/src/analytics/eventUtils.ts

@@ -1,13 +1,10 @@
 import type {Event} from '@amplitude/types';
-import {validate} from 'jsonschema';
-import {APIError, APIErrorResponse, APIErrorSchema} from '../errors/apiError';
 import {CLIBaseError} from '../errors/cliBaseError';
-import {UnknownError} from '../errors/unknownError';
 
 export function buildEvent(
   eventName: string,
   properties: Record<string, unknown>,
-  err?: Error
+  err?: CLIBaseError
 ): Event {
   const analyticsData = {
     event_type: eventName,
@@ -20,26 +17,6 @@ export function buildEvent(
   return analyticsData;
 }
 
-function isErrorFromAPI(arg: unknown): arg is APIErrorResponse {
-  try {
-    return validate(arg, APIErrorSchema).valid;
-  } catch (error) {
-    return false;
-  }
-}
-
-export function buildError(arg: unknown) {
-  const isCLIBaseError = arg instanceof CLIBaseError;
-
-  if (isErrorFromAPI(arg)) {
-    return new APIError(arg);
-  } else if (isCLIBaseError) {
-    return arg;
-  } else {
-    return new UnknownError();
-  }
-}
-
 function errorIdentifier(err?: Error) {
   return {
     ...(err && {

packages/cli/commons/src/command/cliCommand.ts

@@ -0,0 +1,42 @@
+import {Command} from '@oclif/core';
+import {CLIBaseError} from '../errors/cliBaseError';
+import {stopSpinner} from '../utils/ux';
+import {wrapError} from '../errors/wrapError';
+import {Trackable} from '../preconditions/trackable';
+
+/**
+ * A base command to standadize error handling, analytic tracking and logging.
+ *
+ * @class CLICommand
+ * @extends {Command}
+ */
+export abstract class CLICommand extends Command {
+  public abstract run(): PromiseLike<any>;
+
+  /**
+   * If you extend or overwrite the catch method in your command class, make sure it returns `return super.catch(err)`
+   *
+   * @param {*} [err]
+   * @see [Oclif Error Handling](https://oclif.io/docs/error_handling)
+   */
+  @Trackable()
+  protected async catch(err?: any): Promise<CLIBaseError | never> {
+    // Debug raw error
+    this.debug(err);
+
+    // Convert all other errors to CLIBaseErrors for consistency
+    const error = wrapError(err);
+
+    // Let oclif handle errors
+    throw error;
+  }
+
+  protected async finally(err: Error | undefined) {
+    try {
+      const success = !(err instanceof Error);
+      stopSpinner({success});
+    } catch {}
+
+    return super.finally(err);
+  }
+}

packages/cli/commons/src/errors/__snapshots__/wrapError.spec.ts.snap

@@ -0,0 +1,8 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`wrapError when the error is coming from the API should prettify error message from API 1`] = `
+"
+Error code: SOMETHING_WENT_WRONG
+Message: Some error message
+Request ID: some id"
+`;

packages/cli/commons/src/errors/apiError.ts

@@ -1,6 +1,6 @@
 import {red} from 'chalk';
-import dedent from 'ts-dedent';
-import {PrintableError, SeverityLevel} from './printableError';
+import {validate} from 'jsonschema';
+import {CLIBaseError} from './cliBaseError';
 
 export interface APIErrorResponse {
   message: string;
@@ -45,31 +45,35 @@ export const AxiosErrorFromAPISchema = {
   },
 };
 
-export class APIError extends PrintableError {
+export class APIError extends CLIBaseError {
   public constructor(
     error: APIErrorResponse | AxiosErrorFromAPI,
     tagLine?: string
   ) {
-    super(SeverityLevel.Error);
-    let errorCode: string;
-    let message: string;
+    super();
     let status: number | undefined;
+    let messageParts = [''];
+    const {errorCode, message, requestID} = this.isFromAxios(error)
+      ? error.response.data
+      : error;
+
     if (this.isFromAxios(error)) {
-      errorCode = error.response.data.errorCode;
-      message = error.response.data.message;
       status = error.response.status;
-    } else {
-      errorCode = error.errorCode;
-      message = error.message;
     }
 
-    this.name = `APIError - ${errorCode}`;
-    this.message = dedent`
-    ${tagLine}
-    ${status ? `Status code: ${status}\n` : ''}
-    Error code: ${red(errorCode)}
-    Message: ${red(message)}
-    `;
+    this.name = 'APIError';
+    if (tagLine) {
+      messageParts.push(tagLine);
+    }
+    if (status) {
+      messageParts.push(`Status code: ${status}`);
+    }
+    this.message = [
+      ...messageParts,
+      `Error code: ${red(errorCode)}`,
+      `Message: ${red(message)}`,
+      `Request ID: ${red(requestID)}`,
+    ].join('\n');
   }
 
   private isFromAxios(
@@ -78,3 +82,15 @@ export class APIError extends PrintableError {
     return 'response' in error;
   }
 }
+
+export function isErrorFromAPI(arg: unknown): arg is APIErrorResponse {
+  try {
+    return validate(arg, APIErrorSchema).valid;
+  } catch (error) {
+    return false;
+  }
+}
+
+export function isAxiosError(error: unknown): error is AxiosErrorFromAPI {
+  return validate(error, AxiosErrorFromAPISchema).valid;
+}

packages/cli/commons/src/errors/cliBaseError.ts

@@ -1,6 +1,65 @@
-export abstract class CLIBaseError extends Error {
+import {CLIError} from '@oclif/core/lib/errors';
+import {Chalk, red, yellow, cyan} from 'chalk';
+
+export enum SeverityLevel {
+  Info = 'info',
+  Warn = 'warn',
+  Error = 'error',
+}
+
+export interface CLIBaseErrorInterface {
+  level?: SeverityLevel;
+  cause?: Error;
+}
+
+interface OClifCLIError extends Omit<CLIError, 'render' | 'bang' | 'oclif'> {}
+
+export class CLIBaseError extends Error implements OClifCLIError {
+  private static readonly defaultSeverity: SeverityLevel = SeverityLevel.Error;
   public name = 'CLI Error';
-  public constructor(message?: string) {
-    super(message);
+
+  public constructor(
+    private error?: string | Error | CLIError,
+    private options?: CLIBaseErrorInterface
+  ) {
+    super(error instanceof Error ? error.message : error, options);
+  }
+
+  public get stack(): string {
+    return super.stack || '';
+  }
+
+  public get severityLevel(): SeverityLevel {
+    return this.options?.level || CLIBaseError.defaultSeverity;
+  }
+
+  /**
+   * Specific to internal oclif error handling
+   */
+  private get oclif() {
+    return this.error instanceof CLIError ? this.error.oclif : {};
+  }
+
+  /**
+   * Used by oclif to pretty print the error
+   */
+  private get bang() {
+    let color: Chalk;
+
+    switch (this.severityLevel) {
+      case SeverityLevel.Info:
+        color = cyan;
+        break;
+
+      case SeverityLevel.Warn:
+        color = yellow;
+        break;
+
+      case SeverityLevel.Error:
+      default:
+        color = red;
+        break;
+    }
+    return color('»');
   }
 }

packages/cli/commons/src/errors/preconditionError.ts

@@ -1,19 +1,16 @@
-import {PrintableError, SeverityLevel} from './printableError';
+import {CLIBaseError, SeverityLevel} from './cliBaseError';
 
 export interface PreconditionErrorOptions {
   category?: string;
   level?: SeverityLevel;
 }
 
-export class PreconditionError extends PrintableError {
-  public constructor(
-    public message: string,
-    public options?: PreconditionErrorOptions
-  ) {
-    super(options?.level || SeverityLevel.Error);
+export class PreconditionError extends CLIBaseError {
+  public constructor(message: string, options?: PreconditionErrorOptions) {
+    super(message, options);
     this.name = 'Precondition Error';
-    if (this.options?.category) {
-      this.name += ` - ${this.options?.category}`;
+    if (options?.category) {
+      this.name += ` - ${options?.category}`;
     }
   }
 }

packages/cli/commons/src/errors/unknownError.ts

@@ -1,14 +1,12 @@
-import {PrintableError, SeverityLevel} from './printableError';
+import {CLIBaseError} from './cliBaseError';
 
-export class UnknownError extends PrintableError {
+export class UnknownError extends CLIBaseError {
   public name = 'Unknown CLI Error';
   public constructor(e?: unknown) {
-    super(SeverityLevel.Error);
-    const error = typeof e === 'string' ? new Error(e) : e;
-    if (error && error instanceof Error) {
-      this.message = error.message;
-      this.stack = error.stack;
-      this.name = `Unknown CLI Error - ${error.name}`;
-    }
+    super(
+      e instanceof Error
+        ? {...e, name: `${UnknownError.name} - ${e.name}`}
+        : undefined
+    );
   }
 }

packages/cli/commons/src/errors/wrapError.spec.ts

@@ -0,0 +1,82 @@
+import stripAnsi from 'strip-ansi';
+import {fancyIt} from '@coveo/cli-commons-dev/testUtils/it';
+import {APIError} from './apiError';
+import {CLIBaseError} from './cliBaseError';
+import {UnknownError} from './unknownError';
+import {wrapError} from './wrapError';
+
+describe('wrapError', () => {
+  let error: CLIBaseError;
+
+  describe('when the error is a string', () => {
+    beforeAll(() => {
+      error = wrapError('this is an error');
+    });
+
+    fancyIt()('should instanciate a CLIBaseError', () => {
+      expect(error).toBeInstanceOf(CLIBaseError);
+    });
+
+    fancyIt()('should store the message', () => {
+      expect(error.message).toBe('this is an error');
+    });
+  });
+
+  describe('when the error is coming from the API', () => {
+    beforeAll(() => {
+      const apiResponse = {
+        message: 'Some error message',
+        errorCode: 'SOMETHING_WENT_WRONG',
+        requestID: 'some id',
+      };
+      error = wrapError(apiResponse);
+    });
+
+    fancyIt()('should instanciate an APIError', () => {
+      expect(error).toBeInstanceOf(APIError);
+    });
+
+    fancyIt()('should prettify error message from API', () => {
+      expect(stripAnsi(error.message)).toMatchSnapshot();
+    });
+  });
+
+  describe('when the error is already a CLIBaseError', () => {
+    const initialError = new CLIBaseError('😱');
+
+    beforeAll(() => {
+      error = wrapError(initialError);
+    });
+
+    fancyIt()('should return the same CLIBaseError instance', () => {
+      expect(error).toBe(error);
+    });
+  });
+
+  describe('when the error is a generic Error', () => {
+    beforeAll(() => {
+      const genericError = new Error('sad error 😥');
+      error = wrapError(genericError);
+    });
+
+    fancyIt()('should instanciate a CLIBaseError', () => {
+      expect(error).toBeInstanceOf(CLIBaseError);
+    });
+
+    fancyIt()('should persist error message', () => {
+      expect(error.message).toBe('sad error 😥');
+    });
+  });
+
+  describe('when the error is neither of the above', () => {
+    const unknownError = {customParameter: 'foo'};
+
+    beforeAll(() => {
+      error = wrapError(unknownError);
+    });
+
+    fancyIt()('should instanciate an UnknownError', () => {
+      expect(error).toBeInstanceOf(UnknownError);
+    });
+  });
+});