feat: oclif error code handling

src/errorHandling.ts

@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2023, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+
+import { SfError } from '@salesforce/core';
+import { OclifError } from '@oclif/core/lib/interfaces/errors.js';
+import { SfCommandError } from './types.js';
+
+/**
+ *
+ * Takes an error and returns an exit code.
+ * Logic:
+ * - If it looks like a gack, use that code (20)
+ * - If it looks like a TypeError, use that code (10)
+ * - use the exitCode if it is a number
+ * - use the code if it is a number, or 1 if it is present not a number
+ * - use the process exitCode
+ * - default to 1
+ */
+export const computeErrorCode = (e: Error | SfError | SfCommandError): number => {
+  // regardless of the exitCode, we'll set gacks and TypeError to a specific exit code
+  if (errorIsGack(e)) {
+    return 20;
+  }
+
+  if (errorIsTypeError(e)) {
+    return 10;
+  }
+
+  if (isOclifError(e) && typeof e.oclif.exit === 'number') {
+    return e.oclif.exit;
+  }
+
+  if ('exitCode' in e && typeof e.exitCode === 'number') {
+    return e.exitCode;
+  }
+
+  if ('code' in e) {
+    return typeof e.code !== 'number' ? 1 : e.code;
+  }
+
+  return process.exitCode ?? 1;
+};
+
+/** identifies gacks via regex.  Searches the error message, stack, and recursively checks the cause chain */
+export const errorIsGack = (error: Error | SfError): boolean => {
+  /** see test for samples  */
+  const gackRegex = /\d{9,}-\d{3,} \(-?\d{7,}\)/;
+  return (
+    gackRegex.test(error.message) ||
+    (typeof error.stack === 'string' && gackRegex.test(error.stack)) ||
+    // recurse down through the error cause tree to find a gack
+    ('cause' in error && error.cause instanceof Error && errorIsGack(error.cause))
+  );
+};
+
+/** identifies TypeError.  Searches the error message, stack, and recursively checks the cause chain */
+export const errorIsTypeError = (error: Error | SfError): boolean =>
+  error instanceof TypeError ||
+  error.name === 'TypeError' ||
+  error.message.includes('TypeError') ||
+  Boolean(error.stack?.includes('TypeError')) ||
+  ('cause' in error && error.cause instanceof Error && errorIsTypeError(error.cause));
+
+/** custom typeGuard for handling the fact the SfCommand doesn't know about oclif error structure */
+const isOclifError = <T extends Error | SfError | SfCommandError>(e: T): e is T & OclifError =>
+  'oclif' in e ? true : false;

src/sfCommand.ts

@@ -25,6 +25,7 @@ import { formatActions, formatError } from './errorFormatting.js';
 import { StandardColors } from './ux/standardColors.js';
 import { confirm, secretPrompt, PromptInputs } from './ux/prompts.js';
 import { removeEmpty } from './util.js';
+import { computeErrorCode } from './errorHandling.js';
 
 Messages.importMessagesDirectoryFromMetaUrl(import.meta.url);
 const messages = Messages.loadMessages('@salesforce/sf-plugins-core', 'messages');
@@ -378,10 +379,10 @@ export abstract class SfCommand<T> extends Command {
     this.spinner.stop(StandardColors.error('Error'));
     // transform an unknown error into one that conforms to the interface
 
-    // @ts-expect-error because exitCode is not on Error
     // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
-    const codeFromError = (error.oclif?.exit as number | undefined) ?? (error.exitCode as number | undefined) ?? 1;
-    process.exitCode ??= codeFromError;
+    // const codeFromError = (error.oclif?.exit as number | undefined) ?? (error.exitCode as number | undefined) ?? 1;
+    const codeFromError = computeErrorCode(error);
+    process.exitCode = codeFromError;
 
     const sfErrorProperties = removeEmpty({
       code: codeFromError,

test/unit/errorHandling.test.ts

@@ -0,0 +1,124 @@
+/*
+ * Copyright (c) 2023, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+import { expect } from 'chai';
+import { SfError } from '@salesforce/core';
+import { computeErrorCode, errorIsGack, errorIsTypeError } from '../../src/errorHandling.js';
+
+describe('typeErrors', () => {
+  let typeError: Error;
+
+  before(() => {
+    try {
+      const n = null;
+      // @ts-expect-error I know it's wrong, I need an error!
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-call
+      n.f();
+    } catch (e) {
+      if (e instanceof TypeError) {
+        typeError = e;
+      }
+    }
+  });
+  it('matches on TypeError as error name', () => {
+    expect(errorIsTypeError(typeError)).to.be.true;
+  });
+
+  it('matches on TypeError in stack', () => {
+    const e = new Error('some error');
+    e.stack = e.stack + typeError.name;
+    expect(errorIsTypeError(e)).to.be.true;
+  });
+
+  it('matches on TypeError in stack (check against false positive)', () => {
+    const e = new Error('some error');
+    expect(errorIsTypeError(e)).to.be.false;
+  });
+
+  it('matches on TypeError as cause', () => {
+    const error = new SfError('some error', 'testError', [], 44, typeError);
+    expect(errorIsTypeError(error)).to.be.true;
+  });
+});
+describe('gacks', () => {
+  const realGackSamples = [
+    '963190677-320016 (165202460)',
+    '1662648879-55786 (-1856191902)',
+    '694826414-169428 (2085174272)',
+    '1716315817-543601 (74920438)',
+    '1035887602-340708 (1781437152)',
+    '671603042-121307 (-766503277)',
+    '396937656-5973 (-766503277)',
+    '309676439-91665 (-153174221)',
+    '956661320-295482 (2000727581)',
+    '1988392611-333742 (1222029414)',
+    '1830254280-281143 (331700540)',
+  ];
+
+  it('says true for sample gacks', () => {
+    realGackSamples.forEach((gack) => {
+      expect(errorIsGack(new SfError(gack))).to.be.true;
+    });
+  });
+
+  it('error in stack', () => {
+    const error = new SfError('some error');
+    error.stack = realGackSamples[0];
+    expect(errorIsGack(error)).to.be.true;
+  });
+
+  it('error in sfError cause', () => {
+    const error = new SfError('some error', 'testError', [], 44, new Error(realGackSamples[0]));
+    expect(errorIsGack(error)).to.be.true;
+  });
+});
+
+describe('precedence', () => {
+  it('oclif beats normal exit code', () => {
+    const e = new SfError('foo', 'foo', [], 44, undefined);
+    // @ts-expect-error doesn't know about oclif
+    e.oclif = {
+      exit: 99,
+    };
+    expect(computeErrorCode(e)).to.equal(99);
+  });
+  it('oclif vs. normal exit code, but oclif has undefined', () => {
+    const e = new SfError('foo', 'foo', [], 44, undefined);
+    // @ts-expect-error doesn't know about oclif
+    e.oclif = {};
+    expect(computeErrorCode(e)).to.equal(44);
+  });
+  it('oclif vs. normal exit code, but oclif has 0', () => {
+    const e = new SfError('foo', 'foo', [], 44, undefined);
+    // @ts-expect-error doesn't know about oclif
+    e.oclif = {
+      exit: 0,
+    };
+    expect(computeErrorCode(e)).to.equal(0);
+  });
+  it('gack beats oclif and normal exit code', () => {
+    const e = new SfError(
+      'for a good time call Salesforce Support and ask for 1830254280-281143 (867530999)',
+      'foo',
+      [],
+      44,
+      undefined
+    );
+    // @ts-expect-error doesn't know about oclif
+    e.oclif = {
+      exit: 99,
+    };
+    expect(computeErrorCode(e)).to.equal(20);
+  });
+  it('type error beats oclif and normal exit code', () => {
+    const e = new SfError('TypeError: stop using as any!', 'TypeError', [], 44, undefined);
+    // @ts-expect-error doesn't know about oclif
+    e.oclif = {
+      exit: 99,
+    };
+    expect(computeErrorCode(e)).to.equal(10);
+  });
+});