feat: improved sfError

feat: improved sfError

src/messages.ts

@@ -363,14 +363,14 @@ export class Messages<T extends string> {
     }
 
     if (!packageName) {
-      const errMessage = `Invalid or missing package.json file at '${moduleMessagesDirPath}'. If not using a package.json, pass in a packageName.`;
+      const message = `Invalid or missing package.json file at '${moduleMessagesDirPath}'. If not using a package.json, pass in a packageName.`;
       try {
         packageName = asString(ensureJsonMap(Messages.readFile(path.join(moduleMessagesDirPath, 'package.json'))).name);
         if (!packageName) {
-          throw new NamedError('MissingPackageName', errMessage);
+          throw SfError.create({ message, name: 'MissingPackageName' });
         }
       } catch (err) {
-        throw new NamedError('MissingPackageName', errMessage, err as Error);
+        throw SfError.create({ message, name: 'MissingPackageName', cause: err });
       }
     }
 

src/sfError.ts

@@ -5,9 +5,21 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
-import { NamedError } from '@salesforce/kit';
 import { AnyJson, hasString, isString, JsonMap } from '@salesforce/ts-types';
 
+export type SfErrorOptions<T extends ErrorDataProperties = ErrorDataProperties> = {
+  message: string;
+  exitCode?: number;
+  name?: string;
+  data?: T;
+  /** pass an Error.  For convenience in catch blocks, code will check that it is, in fact, an Error */
+  cause?: unknown;
+  context?: string;
+  actions?: string[];
+};
+
+type ErrorDataProperties = AnyJson;
+
 /**
  * A generalized sfdx error which also contains an action. The action is used in the
  * CLI to help guide users past the error.
@@ -24,7 +36,8 @@ import { AnyJson, hasString, isString, JsonMap } from '@salesforce/ts-types';
  * throw new SfError(message.getMessage('myError'), 'MyErrorName');
  * ```
  */
-export class SfError<T = unknown> extends NamedError {
+export class SfError<T extends ErrorDataProperties = ErrorDataProperties> extends Error {
+  public readonly name: string;
   /**
    * Action messages. Hints to the users regarding what can be done to fix related issues.
    */
@@ -59,13 +72,17 @@ export class SfError<T = unknown> extends NamedError {
    */
   public constructor(
     message: string,
-    name?: string,
+    name = 'SfError',
     actions?: string[],
     exitCodeOrCause?: number | Error,
-    cause?: Error
+    cause?: unknown
   ) {
-    cause = exitCodeOrCause instanceof Error ? exitCodeOrCause : cause;
-    super(name ?? 'SfError', message || name, cause);
+    if (typeof cause !== 'undefined' && !(cause instanceof Error)) {
+      throw new TypeError(`The cause, if provided, must be an instance of Error. Received: ${typeof cause}`);
+    }
+    super(message);
+    this.name = name;
+    this.cause = exitCodeOrCause instanceof Error ? exitCodeOrCause : cause;
     this.actions = actions;
     if (typeof exitCodeOrCause === 'number') {
       this.exitCode = exitCodeOrCause;
@@ -74,35 +91,53 @@ export class SfError<T = unknown> extends NamedError {
     }
   }
 
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  public get code(): any {
+  public get code(): string {
     return this.#code ?? this.name;
   }
 
   public set code(code: string) {
     this.#code = code;
   }
 
+  /** like the constructor, but takes an typed object and let you also set context and data properties */
+  public static create<T extends ErrorDataProperties = ErrorDataProperties>(inputs: SfErrorOptions<T>): SfError<T> {
+    const error = new SfError<T>(inputs.message, inputs.name, inputs.actions, inputs.exitCode, inputs.cause);
+    error.data = inputs.data;
+    error.context = inputs.context;
+    return error;
+  }
   /**
    * Convert an Error to an SfError.
    *
    * @param err The error to convert.
    */
-  public static wrap(err: Error | string): SfError {
+  public static wrap<T extends ErrorDataProperties = ErrorDataProperties>(err: unknown): SfError<T> {
     if (isString(err)) {
-      return new SfError(err);
+      return new SfError<T>(err);
     }
 
     if (err instanceof SfError) {
-      return err;
+      return err as SfError<T>;
     }
 
-    const sfError = new SfError(err.message, err.name, undefined, err);
+    const sfError =
+      err instanceof Error
+        ? // a basic error with message and name.  We make it the cause to preserve any other properties
+          SfError.create<T>({
+            message: err.message,
+            name: err.name,
+            cause: err,
+          })
+        : // ok, something was throws that wasn't error or string.  Convert it to an Error that preserves the information as the cause and wrap that.
+          SfError.wrap<T>(
+            new Error(`SfError.wrap received type ${typeof err} but expects type Error or string`, { cause: err })
+          );
 
     // If the original error has a code, use that instead of name.
     if (hasString(err, 'code')) {
       sfError.code = err.code;
     }
+
     return sfError;
   }
 
@@ -130,24 +165,13 @@ export class SfError<T = unknown> extends NamedError {
    * Convert an {@link SfError} state to an object. Returns a plain object representing the state of this error.
    */
   public toObject(): JsonMap {
-    const obj: JsonMap = {
+    return {
       name: this.name,
       message: this.message ?? this.name,
       exitCode: this.exitCode,
       actions: this.actions,
+      ...(this.context ? { context: this.context } : {}),
+      ...(this.data ? { data: this.data } : {}),
     };
-
-    if (this.context) {
-      obj.context = this.context;
-    }
-
-    if (this.data) {
-      // DANGER: data was previously typed as `unknown` and this assertion was here on the toObject.
-      // TODO in next major release: put proper type constraint on SfError.data to something that can serialize
-      // while we're making breaking changes, provide a more definite type for toObject
-      obj.data = this.data as AnyJson;
-    }
-
-    return obj;
   }
 }

test/tsconfig.json

@@ -5,6 +5,10 @@
     "noEmit": true,
     "skipLibCheck": true,
     "resolveJsonModule": true,
-    "esModuleInterop": true
+    "esModuleInterop": true,
+    "lib": ["ES2022"],
+    "module": "Node16",
+    "moduleResolution": "Node16",
+    "target": "ES2022"
   }
 }

test/unit/org/scratchOrgInfoApiTest.ts

@@ -192,7 +192,7 @@ describe('requestScratchOrgCreation', () => {
         expect.fail('should have thrown SfError');
       }
       expect(error).to.exist;
-      expect(error).to.have.keys(['cause', 'name', 'actions', 'exitCode']);
+      expect(error).to.include.keys(['cause', 'name', 'actions', 'exitCode']);
       expect((error as Error).toString()).to.include('SignupDuplicateSettingsSpecifiedError');
     }
   });
@@ -216,7 +216,7 @@ describe('requestScratchOrgCreation', () => {
         expect.fail('should have thrown SfError');
       }
       expect(error).to.exist;
-      expect(error).to.have.keys(['cause', 'name', 'actions', 'exitCode']);
+      expect(error).to.include.keys(['cause', 'name', 'actions', 'exitCode']);
       expect((error as Error).toString()).to.include(messages.getMessage('DeprecatedPrefFormat'));
     }
   });