refactor(cli): optimize error handling in flag parsing (#36810)

This commit is contained in:
Yunlu Wen 2026-05-29 15:39:26 +08:00 committed by GitHub
parent a392a72960
commit 5070cc9668
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 185 additions and 100 deletions

View File

@ -1,5 +1,5 @@
import { Flags } from '../../../framework/flags.js'
import { formatted } from '../../../framework/output.js'
import { formatted, OutputFormat } from '../../../framework/output.js'
import { DifyCommand } from '../../_shared/dify-command.js'
import { httpRetryFlag } from '../../_shared/global-flags.js'
import { runCreateMember } from './run.js'
@ -24,7 +24,7 @@ export default class CreateMember extends DifyCommand {
description: 'workspace id (overrides DIFY_WORKSPACE_ID and stored default)',
}),
'http-retry': httpRetryFlag,
'output': Flags.string({ char: 'o', description: 'output format (json|yaml|name|text)', default: '' }),
'output': Flags.outputFormat({ options: [OutputFormat.JSON, OutputFormat.YAML, OutputFormat.NAME, OutputFormat.TEXT], default: '' }),
}
async run(argv: string[]) {

View File

@ -1,5 +1,5 @@
import { Args, Flags } from '../../../framework/flags.js'
import { formatted } from '../../../framework/output.js'
import { formatted, OutputFormat } from '../../../framework/output.js'
import { DifyCommand } from '../../_shared/dify-command.js'
import { httpRetryFlag } from '../../_shared/global-flags.js'
import { runDeleteMember } from './run.js'
@ -23,7 +23,7 @@ export default class DeleteMember extends DifyCommand {
description: 'workspace id (overrides DIFY_WORKSPACE_ID and stored default)',
}),
'http-retry': httpRetryFlag,
'output': Flags.string({ char: 'o', description: 'output format (json|yaml|name|text)', default: '' }),
'output': Flags.outputFormat({ options: [OutputFormat.JSON, OutputFormat.YAML, OutputFormat.NAME, OutputFormat.TEXT], default: '' }),
'yes': Flags.boolean({ char: 'y', description: 'skip confirmation prompt', default: false }),
}

View File

@ -1,5 +1,5 @@
import { Args, Flags } from '../../../framework/flags.js'
import { formatted } from '../../../framework/output.js'
import { formatted, OutputFormat } from '../../../framework/output.js'
import { DifyCommand } from '../../_shared/dify-command.js'
import { httpRetryFlag } from '../../_shared/global-flags.js'
import { runDescribeApp } from './run.js'
@ -20,7 +20,7 @@ export default class DescribeApp extends DifyCommand {
static override flags = {
'workspace': Flags.string({ description: 'workspace id (overrides DIFY_WORKSPACE_ID and stored default)' }),
'http-retry': httpRetryFlag,
'output': Flags.string({ char: 'o', description: 'output format (json|yaml|text)', default: '' }),
'output': Flags.outputFormat({ options: [OutputFormat.JSON, OutputFormat.YAML, OutputFormat.TEXT], default: '' }),
'refresh': Flags.boolean({ description: 'bypass app-info cache and fetch fresh', default: false }),
}

View File

@ -1,6 +1,6 @@
import type { AppMode } from '@dify/contracts/api/openapi/types.gen'
import { Args, Flags } from '../../../framework/flags.js'
import { table } from '../../../framework/output.js'
import { OutputFormat, table } from '../../../framework/output.js'
import { DifyCommand } from '../../_shared/dify-command.js'
import { httpRetryFlag } from '../../_shared/global-flags.js'
import { runGetApp } from './run.js'
@ -42,7 +42,7 @@ export default class GetApp extends DifyCommand {
'name': Flags.string({ description: 'filter by app name (server-side substring)' }),
'tag': Flags.string({ description: 'filter by tag name (server-side exact match)' }),
'http-retry': httpRetryFlag,
'output': Flags.string({ char: 'o', description: 'output format (json|yaml|name|wide)', default: '' }),
'output': Flags.outputFormat({ options: [OutputFormat.JSON, OutputFormat.YAML, OutputFormat.NAME, OutputFormat.WIDE], default: '' }),
}
async run(argv: string[]) {

View File

@ -1,5 +1,5 @@
import { Flags } from '../../../framework/flags.js'
import { table } from '../../../framework/output.js'
import { OutputFormat, table } from '../../../framework/output.js'
import { DifyCommand } from '../../_shared/dify-command.js'
import { httpRetryFlag } from '../../_shared/global-flags.js'
import { runGetMember } from './run.js'
@ -23,7 +23,7 @@ export default class GetMember extends DifyCommand {
'page': Flags.integer({ description: 'page number', default: 1 }),
'limit': Flags.string({ description: 'page size [1..200]' }),
'http-retry': httpRetryFlag,
'output': Flags.string({ char: 'o', description: 'output format (json|yaml|name|wide)', default: '' }),
'output': Flags.outputFormat({ options: [OutputFormat.JSON, OutputFormat.YAML, OutputFormat.NAME, OutputFormat.WIDE], default: '' }),
}
async run(argv: string[]) {

View File

@ -1,5 +1,5 @@
import { Flags } from '../../../framework/flags.js'
import { raw, table } from '../../../framework/output.js'
import { OutputFormat, raw, table } from '../../../framework/output.js'
import { DifyCommand } from '../../_shared/dify-command.js'
import { httpRetryFlag } from '../../_shared/global-flags.js'
import { runGetWorkspace } from './run.js'
@ -15,7 +15,7 @@ export default class GetWorkspace extends DifyCommand {
static override flags = {
'http-retry': httpRetryFlag,
'output': Flags.string({ char: 'o', description: 'output format (json|yaml|name|wide)', default: '' }),
'output': Flags.outputFormat({ options: [OutputFormat.JSON, OutputFormat.YAML, OutputFormat.NAME, OutputFormat.WIDE], default: '' }),
}
async run(argv: string[]) {

View File

@ -1,4 +1,5 @@
import { Args, Flags } from '../../../framework/flags.js'
import { OutputFormat } from '../../../framework/output.js'
import { DifyCommand } from '../../_shared/dify-command.js'
import { httpRetryFlag } from '../../_shared/global-flags.js'
import { resumeApp } from './run.js'
@ -25,7 +26,7 @@ export default class ResumeApp extends DifyCommand {
'with-history': Flags.boolean({ description: 'Replay executed-node history before attaching to live stream.', default: false }),
'stream': Flags.boolean({ description: 'Print output live as tokens/events arrive. Default: collect and print at end.', default: false }),
'think': Flags.boolean({ description: 'Show model thinking/reasoning when available. Strips <think>...</think> blocks silently by default; with --think, thinking is printed to stderr.', default: false }),
'output': Flags.string({ char: 'o', description: 'output format (json|yaml|text)', default: '' }),
'output': Flags.outputFormat({ options: [OutputFormat.JSON, OutputFormat.YAML, OutputFormat.TEXT], default: '' }),
'http-retry': httpRetryFlag,
}

View File

@ -1,4 +1,5 @@
import { Args, Flags } from '../../../framework/flags.js'
import { OutputFormat } from '../../../framework/output.js'
import { DifyCommand } from '../../_shared/dify-command.js'
import { httpRetryFlag } from '../../_shared/global-flags.js'
import { agentGuide } from './guide.js'
@ -32,7 +33,7 @@ export default class RunApp extends DifyCommand {
'stream': Flags.boolean({ description: 'Print output live as tokens/events arrive (default: collect and print at end)', default: false }),
'think': Flags.boolean({ description: 'Show model thinking/reasoning when available. Strips <think>...</think> blocks silently by default; with --think, thinking is printed to stderr.', default: false }),
'http-retry': httpRetryFlag,
'output': Flags.string({ char: 'o', description: 'Output format (json|yaml|text)', default: '' }),
'output': Flags.outputFormat({ options: [OutputFormat.JSON, OutputFormat.YAML, OutputFormat.TEXT], default: '' }),
}
async run(argv: string[]): Promise<void> {

View File

@ -1,5 +1,5 @@
import { Args, Flags } from '../../../framework/flags.js'
import { formatted } from '../../../framework/output.js'
import { formatted, OutputFormat } from '../../../framework/output.js'
import { DifyCommand } from '../../_shared/dify-command.js'
import { httpRetryFlag } from '../../_shared/global-flags.js'
import { runSetMember } from './run.js'
@ -27,7 +27,7 @@ export default class SetMember extends DifyCommand {
description: 'workspace id (overrides DIFY_WORKSPACE_ID and stored default)',
}),
'http-retry': httpRetryFlag,
'output': Flags.string({ char: 'o', description: 'output format (json|yaml|name|text)', default: '' }),
'output': Flags.outputFormat({ options: [OutputFormat.JSON, OutputFormat.YAML, OutputFormat.NAME, OutputFormat.TEXT], default: '' }),
}
async run(argv: string[]) {

View File

@ -1,5 +1,5 @@
import { Flags } from '../../framework/flags.js'
import { formatted, raw, stringifyOutput } from '../../framework/output.js'
import { formatted, OutputFormat, raw, stringifyOutput } from '../../framework/output.js'
import { colorEnabled } from '../../sys/io/color.js'
import { realStreams } from '../../sys/io/streams'
import { versionInfo } from '../../version/info.js'
@ -20,11 +20,7 @@ export default class Version extends DifyCommand {
]
static override flags = {
'output': Flags.string({
char: 'o',
description: 'output format (text|json|yaml)',
default: '',
}),
'output': Flags.outputFormat({ options: [OutputFormat.TEXT, OutputFormat.JSON, OutputFormat.YAML], default: '' }),
'client': Flags.boolean({ description: 'skip server probe' }),
'short': Flags.boolean({ description: 'print only the client semver' }),
'check-compat': Flags.boolean({

View File

@ -8,8 +8,8 @@ import {
} from './codes.js'
describe('error codes', () => {
it('has 18 codes (parity with internal/api/errors)', () => {
expect(ALL_ERROR_CODES).toHaveLength(18)
it('has correct number codes (parity with internal/api/errors)', () => {
expect(ALL_ERROR_CODES).toHaveLength(Object.keys(CODE_TO_EXIT_MAP).length)
})
it('has the expected ExitCode buckets', () => {

View File

@ -17,6 +17,7 @@ export const ErrorCode = {
Server4xxOther: 'server_4xx_other',
ClientError: 'client_error',
Unknown: 'unknown',
IllegalArgumentError: 'illegal_argument',
} as const
export type ErrorCodeValue = (typeof ErrorCode)[keyof typeof ErrorCode]
@ -50,6 +51,7 @@ const CODE_TO_EXIT: Readonly<Record<ErrorCodeValue, ExitCodeValue>> = {
server_4xx_other: ExitCode.Generic,
client_error: ExitCode.Generic,
unknown: ExitCode.Generic,
illegal_argument: ExitCode.Usage,
}
export function exitFor(code: string): ExitCodeValue {

View File

@ -0,0 +1,35 @@
import type { FlagDefinition } from './types.js'
import { describe, expect, it } from 'vitest'
import { OutputFormatNotSupportedError, UnsupportedArgValueError } from './errors.js'
describe('OutputFormatNotSupportedError', () => {
it('states the offending format in the message', () => {
const err = new OutputFormatNotSupportedError('csv')
expect(err.message).toBe('format csv is not supported by this command')
})
})
describe('UnsupportedArgValueError', () => {
it('includes both long and short option labels when a char exists', () => {
const def: FlagDefinition = { type: 'string', description: 'output', char: 'o', options: ['json', 'yaml'] }
const err = new UnsupportedArgValueError('output', def, 'csv')
expect(err.message).toBe('illegal value csv for flag --output / -o')
})
it('omits the short option label when the flag has no char', () => {
const def: FlagDefinition = { type: 'string', description: 'app mode', options: ['chat', 'workflow'] }
const err = new UnsupportedArgValueError('mode', def, 'chatbot')
expect(err.message).toBe('illegal value chatbot for flag --mode')
})
it('lists supported values in the hint', () => {
const def: FlagDefinition = { type: 'string', description: 'app mode', options: ['chat', 'workflow'] }
expect(new UnsupportedArgValueError('mode', def, 'chatbot').hint).toBe('supported value: chat, workflow')
})
it('leaves the hint empty when the flag declares no options', () => {
const def: FlagDefinition = { type: 'string', description: 'app mode' }
expect(new UnsupportedArgValueError('mode', def, 'chatbot').hint).toBe('')
})
})

View File

@ -0,0 +1,23 @@
import type { FlagDefinition } from './types'
import { BaseError } from '../errors/base'
import { ErrorCode } from '../errors/codes'
export class OutputFormatNotSupportedError extends BaseError {
constructor(format: string) {
super({
code: ErrorCode.IllegalArgumentError,
message: `format ${format} is not supported by this command`,
})
}
}
export class UnsupportedArgValueError extends BaseError {
constructor(flagName: string, flagDef: FlagDefinition, givenValue: string) {
const flagLabel = flagDef.char ? `--${flagName} / -${flagDef.char}` : `--${flagName}`
super({
code: ErrorCode.IllegalArgumentError,
message: `illegal value ${givenValue} for flag ${flagLabel}`,
hint: flagDef.options ? `supported value: ${flagDef.options.join(', ')}` : '',
})
}
}

View File

@ -1,4 +1,5 @@
import { describe, expect, it } from 'vitest'
import { UnsupportedArgValueError } from './errors.js'
import { Args, Flags, parseArgv } from './flags.js'
const meta = {
@ -190,13 +191,13 @@ describe('parseArgv', () => {
it('rejects an invalid option value (space form)', () => {
expect(() => parseArgv(['--mode', 'chatbot'], metaWithOptions)).toThrow(
'--mode must be one of: chat, workflow, completion',
UnsupportedArgValueError,
)
})
it('rejects an invalid option value (= form)', () => {
expect(() => parseArgv(['--mode=chatbot'], metaWithOptions)).toThrow(
'--mode must be one of: chat, workflow, completion',
UnsupportedArgValueError,
)
})
})

View File

@ -1,6 +1,12 @@
import type { ArgDefinition, CommandMeta, FlagDefinition, ParsedArgs, ParsedFlags } from './types.js'
import { UnsupportedArgValueError } from './errors.js'
function stringFlag<const Opts extends { description: string, char?: string, default?: string, multiple?: boolean, helpGroup?: string, options?: readonly string[] }>(
function stringFlag<const Opts extends {
description: string
char?: string
default?: string
options?: readonly string[]
}>(
opts: Opts,
): FlagDefinition<string> {
return {
@ -10,7 +16,19 @@ function stringFlag<const Opts extends { description: string, char?: string, def
}
}
function stringRepeatedFlag<const Opts extends { description: string, char?: string, default?: string[], multiple?: boolean, helpGroup?: string }>(
function outputFormatFlag<const Opts extends { options: readonly string[], default?: string }>(
opts: Opts,
): FlagDefinition<string> {
return {
type: 'string',
description: `output format (${opts.options.join('|')})`,
char: 'o',
multiple: false,
...opts,
}
}
function stringRepeatedFlag<const Opts extends { description: string, char?: string, default?: string[], multiple?: boolean }>(
opts: Opts,
): FlagDefinition<string[]> {
return {
@ -20,11 +38,11 @@ function stringRepeatedFlag<const Opts extends { description: string, char?: str
}
}
function booleanFlag(opts: { description: string, char?: string, default?: boolean, helpGroup?: string }): FlagDefinition<boolean> {
function booleanFlag(opts: { description: string, char?: string, default?: boolean }): FlagDefinition<boolean> {
return { type: 'boolean', ...opts }
}
function integerFlag<const Opts extends { description: string, char?: string, default?: number, helpGroup?: string }>(
function integerFlag<const Opts extends { description: string, char?: string, default?: number }>(
opts: Opts,
): FlagDefinition<Opts extends { default: number } ? number : number | undefined> {
return { type: 'integer', ...opts } as FlagDefinition<Opts extends { default: number } ? number : number | undefined>
@ -35,6 +53,7 @@ export const Flags = {
stringArray: stringRepeatedFlag,
boolean: booleanFlag,
integer: integerFlag,
outputFormat: outputFormatFlag,
}
function stringArg<const Opts extends { description: string, required?: boolean }>(
@ -91,7 +110,32 @@ function resolveByChar(char: string, meta: CommandMeta): [name: string, def: Fla
function validateFlagOptions(name: string, raw: string, def: FlagDefinition): void {
if (def.options !== undefined && !def.options.includes(raw))
throw new Error(`--${name} must be one of: ${def.options.join(', ')}`)
throw new UnsupportedArgValueError(name, def, raw)
}
type ResolvedFlag = { name: string, def: FlagDefinition, label: string, inlineRaw: string | undefined }
function resolveToken(token: string, meta: CommandMeta): ResolvedFlag | null {
if (token.startsWith('--')) {
const eqIdx = token.indexOf('=')
const name = eqIdx !== -1 ? token.slice(2, eqIdx) : token.slice(2)
const inlineRaw = eqIdx !== -1 ? token.slice(eqIdx + 1) : undefined
const def = meta.flags[name]
if (!def)
throw new Error(`unknown flag: --${name}`)
return { name, def, label: `--${name}`, inlineRaw }
}
if (token.length === 2 && token[1] !== undefined) {
const char = token[1]
const resolved = resolveByChar(char, meta)
if (!resolved)
throw new Error(`unknown flag: -${char}`)
const [name, def] = resolved
return { name, def, label: `-${char}`, inlineRaw: undefined }
}
return null
}
export function parseArgv(argv: readonly string[], meta: CommandMeta): { args: ParsedArgs, flags: ParsedFlags } {
@ -110,63 +154,38 @@ export function parseArgv(argv: readonly string[], meta: CommandMeta): { args: P
continue
}
if (!pastDoubleDash && token.startsWith('--')) {
const eqIdx = token.indexOf('=')
let name: string
let rawValue: string | undefined
if (eqIdx !== -1) {
name = token.slice(2, eqIdx)
rawValue = token.slice(eqIdx + 1)
}
else {
name = token.slice(2)
rawValue = undefined
}
const def = meta.flags[name]
if (!def)
throw new Error(`unknown flag: --${name}`)
if (def.type === 'boolean') {
flags[name] = rawValue === undefined ? true : coerceFlagValue(rawValue, def)
}
else if (rawValue !== undefined) {
validateFlagOptions(name, rawValue, def)
accumulateFlagValue(flags, name, coerceFlagValue(rawValue, def), def)
}
else {
i++
const next = i < argv.length ? argv[i] : undefined
if (next === undefined || next.startsWith('-'))
throw new Error(`flag --${name} expects a value`)
validateFlagOptions(name, next, def)
accumulateFlagValue(flags, name, coerceFlagValue(next, def), def)
}
if (pastDoubleDash || !token.startsWith('-')) {
positional.push(token)
continue
}
else if (!pastDoubleDash && token.startsWith('-') && token.length === 2 && token[1] !== undefined) {
const char = token[1]
const resolved = resolveByChar(char, meta)
if (!resolved)
throw new Error(`unknown flag: -${char}`)
const [flagName, def] = resolved
if (def.type === 'boolean') {
flags[flagName] = true
}
else {
i++
const next = i < argv.length ? argv[i] : undefined
if (next === undefined || next.startsWith('-'))
throw new Error(`flag -${char} expects a value`)
const resolved = resolveToken(token, meta)
if (!resolved) {
positional.push(token)
continue
}
accumulateFlagValue(flags, flagName, coerceFlagValue(next, def), def)
}
const { name, def, label, inlineRaw } = resolved
if (def.type === 'boolean') {
flags[name] = inlineRaw === undefined ? true : coerceFlagValue(inlineRaw, def)
continue
}
let raw: string
if (inlineRaw !== undefined) {
raw = inlineRaw
}
else {
positional.push(token)
i++
const next = i < argv.length ? argv[i] : undefined
if (next === undefined || next.startsWith('-'))
throw new Error(`flag ${label} expects a value`)
raw = next
}
validateFlagOptions(name, raw, def)
accumulateFlagValue(flags, name, coerceFlagValue(raw, def), def)
}
const args: ParsedArgs = {}

View File

@ -1,5 +1,6 @@
import type { FormattedPrintable, NamePrintable, TablePrintable } from './output.js'
import { describe, expect, it } from 'vitest'
import { OutputFormatNotSupportedError } from './errors.js'
import {
formatted,
raw,
@ -99,13 +100,12 @@ describe('stringifyOutput — formatted', () => {
json: () => ({}),
}
const out = formatted({ format: 'name', data: noName })
expect(() => stringifyOutput(out)).toThrow('name output requires data.name()')
expect(() => stringifyOutput(out)).toThrow(OutputFormatNotSupportedError)
})
it('unknown format: throws with allowed list', () => {
const out = formatted({ format: 'csv', data: makeFormatted({}) })
expect(() => stringifyOutput(out)).toThrow(/not supported/)
expect(() => stringifyOutput(out)).toThrow(/json, name, text, yaml/)
expect(() => stringifyOutput(out)).toThrow(OutputFormatNotSupportedError)
})
})
@ -175,13 +175,12 @@ describe('stringifyOutput — table', () => {
json: () => [],
}
const out = table({ format: 'name', data: noName })
expect(() => stringifyOutput(out)).toThrow('name output requires data.name()')
expect(() => stringifyOutput(out)).toThrow(OutputFormatNotSupportedError)
})
it('unknown format: throws with allowed list', () => {
const out = table({ format: 'csv', data: makeTable({}) })
expect(() => stringifyOutput(out)).toThrow(/not supported/)
expect(() => stringifyOutput(out)).toThrow(/json, name, wide, yaml/)
expect(() => stringifyOutput(out)).toThrow(OutputFormatNotSupportedError)
})
it('table renders column padding correctly', () => {

View File

@ -1,4 +1,5 @@
import yaml from 'js-yaml'
import { OutputFormatNotSupportedError } from './errors'
export type RawOutput = {
readonly kind: 'raw'
@ -31,6 +32,14 @@ export type JsonPrintable = {
readonly json: () => unknown
}
export const OutputFormat = {
NAME: 'name',
JSON: 'json',
YAML: 'yaml',
TEXT: 'text',
WIDE: 'wide',
} as const
export type TableOutput<TRow extends TablePrintable> = {
readonly kind: 'table'
readonly format: string
@ -77,32 +86,32 @@ export function stringifyOutput(output: CommandOutput): string {
function stringifyFormattedOutput(output: FormattedOutput<FormattedPrintable>): string {
switch (output.format) {
case '':
case 'text':
case OutputFormat.TEXT:
return output.data.text()
case 'json':
case OutputFormat.JSON:
return `${JSON.stringify(output.data.json(), null, 2)}\n`
case 'yaml':
case OutputFormat.YAML:
return yaml.dump(output.data.json(), { indent: 2, lineWidth: -1 })
case 'name':
case OutputFormat.NAME:
return `${toName(output.data)}\n`
default:
throw new Error(`output format ${JSON.stringify(output.format)} not supported, allowed: json, name, text, yaml`)
throw new OutputFormatNotSupportedError(output.format)
}
}
function stringifyTableOutput(output: TableOutput<TablePrintable>): string {
switch (output.format) {
case '':
case 'wide':
case OutputFormat.WIDE:
return renderTable(output)
case 'json':
case OutputFormat.JSON:
return `${JSON.stringify(output.data.json(), null, 2)}\n`
case 'yaml':
case OutputFormat.YAML:
return yaml.dump(output.data.json(), { indent: 2, lineWidth: -1 })
case 'name':
case OutputFormat.NAME:
return `${toName(output.data)}\n`
default:
throw new Error(`output format ${JSON.stringify(output.format)} not supported, allowed: json, name, wide, yaml`)
throw new OutputFormatNotSupportedError(output.format)
}
}
@ -186,7 +195,7 @@ function formatTable(rows: readonly (readonly string[])[]): string {
function toName(data: TablePrintable | FormattedPrintable): string {
if (!isNamePrintable(data))
throw new Error('name output requires data.name()')
throw new OutputFormatNotSupportedError('name')
return data.name()
}

View File

@ -9,7 +9,6 @@ export type FlagDefinition<T extends OptionalArgValueType = OptionalArgValueType
readonly char?: string
readonly default?: ArgValueType
readonly multiple?: boolean
readonly helpGroup?: string
readonly options?: readonly string[]
readonly _flagValue?: T
}