fix: add exit codes to different flag validation errors (#861)

* fix: add exit codes to different flag validation errors * chore: remove unnecessary exit definition * chore: configurable exit codes via CLI pjson * feat: cache exit codes * fix: improve implementation * test: better error code tests * chore: code review * feat: add default exit code --------- Co-authored-by: Mike Donnalley <[email protected]>
oclif · Dec 4, 2023 · 1c841bf · 1c841bf
1 parent 6bd9908
commit 1c841bf
Show file tree

Hide file tree

Showing 14 changed files with 258 additions and 37 deletions.
diff --git a/src/config/cache.ts → src/cache.ts b/src/config/cache.ts → src/cache.ts
@@ -1,7 +1,8 @@
-import {Plugin} from '../interfaces'
+import {PJSON, Plugin} from './interfaces'
 
 type CacheContents = {
   rootPlugin: Plugin
+  exitCodes: PJSON.Plugin['oclif']['exitCodes']
 }
 
 type ValueOf<T> = T[keyof T]
@@ -20,6 +21,7 @@ export default class Cache extends Map<keyof CacheContents, ValueOf<CacheContent
   }
 
   public get(key: 'rootPlugin'): Plugin | undefined
+  public get(key: 'exitCodes'): PJSON.Plugin['oclif']['exitCodes'] | undefined
   public get(key: keyof CacheContents): ValueOf<CacheContents> | undefined {
     return super.get(key)
   }

diff --git a/src/config/config.ts b/src/config/config.ts
@@ -4,6 +4,7 @@ import {arch, userInfo as osUserInfo, release, tmpdir, type} from 'node:os'
 import {join, resolve, sep} from 'node:path'
 import {URL, fileURLToPath} from 'node:url'
 
+import Cache from '../cache'
 import {ux} from '../cli-ux'
 import {parseTheme} from '../cli-ux/theme'
 import {Command} from '../command'
@@ -19,7 +20,6 @@ import {settings} from '../settings'
 import {requireJson, safeReadJson} from '../util/fs'
 import {getHomeDir, getPlatform} from '../util/os'
 import {compact, isProd} from '../util/util'
-import Cache from './cache'
 import PluginLoader from './plugin-loader'
 import {tsPath} from './ts-node'
 import {Debug, collectUsableIds, getCommandIdPermutations} from './util'
@@ -290,7 +290,9 @@ export class Config implements IConfig {
 
     // Cache the root plugin so that we can reference it later when determining if
     // we should skip ts-node registration for an ESM plugin.
-    Cache.getInstance().set('rootPlugin', this.rootPlugin)
+    const cache = Cache.getInstance()
+    cache.set('rootPlugin', this.rootPlugin)
+    cache.set('exitCodes', this.rootPlugin.pjson.oclif.exitCodes ?? {})
 
     this.root = this.rootPlugin.root
     this.pjson = this.rootPlugin.pjson

diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts
@@ -1,12 +1,12 @@
 import {join, relative as pathRelative, sep} from 'node:path'
 import * as TSNode from 'ts-node'
 
+import Cache from '../cache'
 import {memoizedWarn} from '../errors'
 import {Plugin, TSConfig} from '../interfaces'
 import {settings} from '../settings'
 import {existsSync, readTSConfig} from '../util/fs'
 import {isProd} from '../util/util'
-import Cache from './cache'
 import {Debug} from './util'
 // eslint-disable-next-line new-cap
 const debug = Debug('ts-node')

diff --git a/src/errors/errors/cli.ts b/src/errors/errors/cli.ts
@@ -3,6 +3,7 @@ import cs from 'clean-stack'
 import indent from 'indent-string'
 import wrap from 'wrap-ansi'
 
+import Cache from '../../cache'
 import {OclifError, PrettyPrintableError} from '../../interfaces/errors'
 import {errtermwidth} from '../../screen'
 import {config} from '../config'
@@ -16,7 +17,7 @@ export function addOclifExitCode(error: Record<string, any>, options?: {exit?: f
     ;(error as unknown as OclifError).oclif = {}
   }
 
-  error.oclif.exit = options?.exit === undefined ? 2 : options.exit
+  error.oclif.exit = options?.exit === undefined ? Cache.getInstance().get('exitCodes')?.default ?? 2 : options.exit
   return error as OclifError
 }
 

diff --git a/src/interfaces/parser.ts b/src/interfaces/parser.ts
@@ -9,6 +9,7 @@ export type CLIParseErrorOptions = {
     input?: ParserInput
     output?: ParserOutput
   }
+  exit?: number
 }
 
 export type OutputArgs<T extends ParserInput['args']> = {[P in keyof T]: any}

diff --git a/src/interfaces/pjson.ts b/src/interfaces/pjson.ts
@@ -25,6 +25,15 @@ export namespace PJSON {
       default?: string
       description?: string
       devPlugins?: string[]
+      exitCodes?: {
+        default?: number
+        failedFlagParsing?: number
+        failedFlagValidation?: number
+        invalidArgsSpec?: number
+        nonExistentFlag?: number
+        requiredArgs?: number
+        unexpectedArgs?: number
+      }
       flexibleTaxonomy?: boolean
       helpClass?: string
       helpOptions?: HelpOptions

diff --git a/src/parser/errors.ts b/src/parser/errors.ts
@@ -1,11 +1,10 @@
 import chalk from 'chalk'
 
+import Cache from '../cache'
 import {renderList} from '../cli-ux/list'
 import {CLIError} from '../errors'
-import {Flag, OptionFlag} from '../interfaces'
-import {Arg, ArgInput, CLIParseErrorOptions} from '../interfaces/parser'
+import {Arg, ArgInput, CLIParseErrorOptions, OptionFlag} from '../interfaces/parser'
 import {uniq} from '../util/util'
-import {flagUsages} from './help'
 
 export {CLIError} from '../errors'
 
@@ -21,15 +20,15 @@ export class CLIParseError extends CLIError {
 
   constructor(options: CLIParseErrorOptions & {message: string}) {
     options.message += '\nSee more help with --help'
-    super(options.message)
+    super(options.message, {exit: options.exit})
     this.parse = options.parse
   }
 }
 
 export class InvalidArgsSpecError extends CLIParseError {
   public args: ArgInput
 
-  constructor({args, parse}: CLIParseErrorOptions & {args: ArgInput}) {
+  constructor({args, exit, parse}: CLIParseErrorOptions & {args: ArgInput}) {
     let message = 'Invalid argument spec'
     const namedArgs = Object.values(args).filter((a) => a.name)
     if (namedArgs.length > 0) {
@@ -41,7 +40,7 @@ export class InvalidArgsSpecError extends CLIParseError {
       message += `:\n${list}`
     }
 
-    super({message, parse})
+    super({exit: Cache.getInstance().get('exitCodes')?.invalidArgsSpec ?? exit, message, parse})
     this.args = args
   }
 }
@@ -51,6 +50,7 @@ export class RequiredArgsError extends CLIParseError {
 
   constructor({
     args,
+    exit,
     flagsWithMultiple,
     parse,
   }: CLIParseErrorOptions & {args: Arg<any>[]; flagsWithMultiple?: string[]}) {
@@ -71,38 +71,27 @@ export class RequiredArgsError extends CLIParseError {
       message += '\nAlternatively, you can use "--" to signify the end of the flags and the beginning of arguments.'
     }
 
-    super({message, parse})
+    super({exit: Cache.getInstance().get('exitCodes')?.requiredArgs ?? exit, message, parse})
     this.args = args
   }
 }
 
-export class RequiredFlagError extends CLIParseError {
-  public flag: Flag<any>
-
-  constructor({flag, parse}: CLIParseErrorOptions & {flag: Flag<any>}) {
-    const usage = renderList(flagUsages([flag], {displayRequired: false}))
-    const message = `Missing required flag:\n${usage}`
-    super({message, parse})
-    this.flag = flag
-  }
-}
-
 export class UnexpectedArgsError extends CLIParseError {
   public args: unknown[]
 
-  constructor({args, parse}: CLIParseErrorOptions & {args: unknown[]}) {
+  constructor({args, exit, parse}: CLIParseErrorOptions & {args: unknown[]}) {
     const message = `Unexpected argument${args.length === 1 ? '' : 's'}: ${args.join(', ')}`
-    super({message, parse})
+    super({exit: Cache.getInstance().get('exitCodes')?.unexpectedArgs ?? exit, message, parse})
     this.args = args
   }
 }
 
 export class NonExistentFlagsError extends CLIParseError {
   public flags: string[]
 
-  constructor({flags, parse}: CLIParseErrorOptions & {flags: string[]}) {
+  constructor({exit, flags, parse}: CLIParseErrorOptions & {flags: string[]}) {
     const message = `Nonexistent flag${flags.length === 1 ? '' : 's'}: ${flags.join(', ')}`
-    super({message, parse})
+    super({exit: Cache.getInstance().get('exitCodes')?.nonExistentFlag ?? exit, message, parse})
     this.flags = flags
   }
 }
@@ -122,11 +111,21 @@ export class ArgInvalidOptionError extends CLIParseError {
 }
 
 export class FailedFlagValidationError extends CLIParseError {
-  constructor({failed, parse}: CLIParseErrorOptions & {failed: Validation[]}) {
+  constructor({exit, failed, parse}: CLIParseErrorOptions & {failed: Validation[]}) {
     const reasons = failed.map((r) => r.reason)
     const deduped = uniq(reasons)
     const errString = deduped.length === 1 ? 'error' : 'errors'
     const message = `The following ${errString} occurred:\n  ${chalk.dim(deduped.join('\n  '))}`
-    super({message, parse})
+    super({exit: Cache.getInstance().get('exitCodes')?.failedFlagValidation ?? exit, message, parse})
+  }
+}
+
+export class FailedFlagParsingError extends CLIParseError {
+  constructor({flag, message}: {flag: string; message: string}) {
+    super({
+      exit: Cache.getInstance().get('exitCodes')?.failedFlagParsing,
+      message: `Parsing --${flag} \n\t${message}`,
+      parse: {},
+    })
   }
 }
diff --git a/src/parser/parse.ts b/src/parser/parse.ts
@@ -19,7 +19,7 @@ import {
   ParsingToken,
 } from '../interfaces/parser'
 import {isTruthy, last, pickBy} from '../util/util'
-import {ArgInvalidOptionError, CLIError, FlagInvalidOptionError} from './errors'
+import {ArgInvalidOptionError, CLIError, FailedFlagParsingError, FlagInvalidOptionError} from './errors'
 
 let debug: any
 try {
@@ -341,8 +341,7 @@ export class Parser<
 
         return await flag.parse(input, ctx, flag)
       } catch (error: any) {
-        error.message = `Parsing --${flag.name} \n\t${error.message}\nSee more help with --help`
-        throw error
+        throw new FailedFlagParsingError({flag: flag.name, message: error.message})
       }
     }
 

diff --git a/src/parser/validate.ts b/src/parser/validate.ts
@@ -14,13 +14,19 @@ export async function validate(parse: {input: ParserInput; output: ParserOutput}
 
   function validateArgs() {
     if (parse.output.nonExistentFlags?.length > 0) {
-      throw new NonExistentFlagsError({flags: parse.output.nonExistentFlags, parse})
+      throw new NonExistentFlagsError({
+        flags: parse.output.nonExistentFlags,
+        parse,
+      })
     }
 
     const maxArgs = Object.keys(parse.input.args).length
     if (parse.input.strict && parse.output.argv.length > maxArgs) {
       const extras = parse.output.argv.slice(maxArgs)
-      throw new UnexpectedArgsError({args: extras, parse})
+      throw new UnexpectedArgsError({
+        args: extras,
+        parse,
+      })
     }
 
     const missingRequiredArgs: Arg<any>[] = []
@@ -32,7 +38,10 @@ export async function validate(parse: {input: ParserInput; output: ParserOutput}
       } else if (hasOptional) {
         // (required arg) check whether an optional has occurred before
         // optionals should follow required, not before
-        throw new InvalidArgsSpecError({args: parse.input.args, parse})
+        throw new InvalidArgsSpecError({
+          args: parse.input.args,
+          parse,
+        })
       }
 
       if (arg.required && !parse.output.args[name] && parse.output.args[name] !== 0) {
@@ -45,7 +54,11 @@ export async function validate(parse: {input: ParserInput; output: ParserOutput}
         .filter(([_, flagDef]) => flagDef.type === 'option' && Boolean(flagDef.multiple))
         .map(([name]) => name)
 
-      throw new RequiredArgsError({args: missingRequiredArgs, flagsWithMultiple, parse})
+      throw new RequiredArgsError({
+        args: missingRequiredArgs,
+        flagsWithMultiple,
+        parse,
+      })
     }
   }
 
@@ -76,7 +89,11 @@ export async function validate(parse: {input: ParserInput; output: ParserOutput}
     const results = await Promise.all(promises)
 
     const failed = results.filter((r) => r.status === 'failed')
-    if (failed.length > 0) throw new FailedFlagValidationError({failed, parse})
+    if (failed.length > 0)
+      throw new FailedFlagValidationError({
+        failed,
+        parse,
+      })
   }
 
   async function resolveFlags(flags: FlagRelationship[]): Promise<Record<string, unknown>> {