From 61582b5612204eafc3fa75b23ad4190a22f40ee0 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Mon, 8 Jan 2024 15:13:56 -0600 Subject: [PATCH] feat: oclif error code handling --- src/errorHandling.ts | 70 ++++++++++++++++++ src/sfCommand.ts | 7 +- test/unit/errorHandling.test.ts | 124 ++++++++++++++++++++++++++++++++ 3 files changed, 198 insertions(+), 3 deletions(-) create mode 100644 src/errorHandling.ts create mode 100644 test/unit/errorHandling.test.ts diff --git a/src/errorHandling.ts b/src/errorHandling.ts new file mode 100644 index 000000000..9560c9dbb --- /dev/null +++ b/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 = (e: T): e is T & OclifError => + 'oclif' in e ? true : false; diff --git a/src/sfCommand.ts b/src/sfCommand.ts index 280c84ccd..a276ac5dc 100644 --- a/src/sfCommand.ts +++ b/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 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, diff --git a/test/unit/errorHandling.test.ts b/test/unit/errorHandling.test.ts new file mode 100644 index 000000000..0546fc7fa --- /dev/null +++ b/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); + }); +});