mirror of
https://github.com/langgenius/dify.git
synced 2026-06-08 09:27:39 +08:00
fix(cli): restore BaseError catch routing post-oclif removal
PR #36328 (remove oclif) dropped the DifyCommand.catch() override that routed BaseError through formatErrorForCli with semantic exit codes. The replacement catch in framework/run.ts wrote raw err.message and always exited 1, losing the code prefix, hint, http_status line, JSON envelope path, and Auth/Usage/VersionCompat exit codes. framework/run.ts: - Add sniffOutputFormat(argv) helper: detects --output / -o (= and space forms), stops at --, first-occurrence-wins. Schema-free so it survives command-construction failures and pre-parse throws. - Rewrite catch block: branch BaseError -> Error -> non-Error. BaseError branch routes through formatErrorForCli({ format: sniffOutputFormat(argv) }) and exits via err.exit(). Explicit return after each process.exit defends against stubbed exits in tests. run/app/sse-collector.ts: - decodeStreamError now unwraps openapi-v1 InvokeError envelopes ({error_type, args, message}) buried inside env.message. Prefers args.description, falls back to inner.message, then raw on shape mismatch. framework/command.ts: - Sort named imports (fix pre-existing lint error). Tests (run.test.ts new, sse-collector.test.ts extended): - 10 sniffOutputFormat cases. - 12 run() catch-routing cases: BaseError human/JSON, Usage/Server5xx exit codes, withRequest method+url in human and JSON, generic Error, non-Error throw, success path, constructor-time BaseError, -- separator. - 5 decodeStreamError unwrap cases. Full suite: 675/675. type-check + lint clean. No subclass changes.
This commit is contained in:
@ -119,6 +119,45 @@ describe('decodeStreamError', () => {
|
||||
const err = decodeStreamError(new Uint8Array())
|
||||
expect(err.message).toMatch(/error event/i)
|
||||
})
|
||||
|
||||
it('unwraps openapi-v1 invoke-error: prefers args.description', () => {
|
||||
const inner = {
|
||||
args: { description: '[models] Error: API request failed with status code 402: Insufficient Balance' },
|
||||
error_type: 'InvokeError',
|
||||
message: 'fallback message',
|
||||
}
|
||||
const env = { message: JSON.stringify(inner), status: 400 }
|
||||
const err = decodeStreamError(enc.encode(JSON.stringify(env)))
|
||||
expect(err.message).toBe(inner.args.description)
|
||||
expect(err.code).toBe('server_4xx_other')
|
||||
expect(err.httpStatus).toBe(400)
|
||||
})
|
||||
|
||||
it('unwraps openapi-v1 invoke-error: falls back to inner.message when no args.description', () => {
|
||||
const inner = { error_type: 'InvokeError', message: 'inner only' }
|
||||
const env = { message: JSON.stringify(inner), status: 500 }
|
||||
const err = decodeStreamError(enc.encode(JSON.stringify(env)))
|
||||
expect(err.message).toBe('inner only')
|
||||
expect(err.code).toBe('server_5xx')
|
||||
})
|
||||
|
||||
it('leaves message untouched when env.message is plain text', () => {
|
||||
const env = { message: 'plain text error', status: 400 }
|
||||
const err = decodeStreamError(enc.encode(JSON.stringify(env)))
|
||||
expect(err.message).toBe('plain text error')
|
||||
})
|
||||
|
||||
it('leaves message untouched when nested JSON lacks error_type', () => {
|
||||
const env = { message: JSON.stringify({ foo: 'bar' }), status: 400 }
|
||||
const err = decodeStreamError(enc.encode(JSON.stringify(env)))
|
||||
expect(err.message).toBe(JSON.stringify({ foo: 'bar' }))
|
||||
})
|
||||
|
||||
it('leaves message untouched on malformed nested JSON starting with {', () => {
|
||||
const env = { message: '{not valid json', status: 400 }
|
||||
const err = decodeStreamError(enc.encode(JSON.stringify(env)))
|
||||
expect(err.message).toBe('{not valid json')
|
||||
})
|
||||
})
|
||||
|
||||
describe('collect — human_input_required', () => {
|
||||
|
||||
@ -157,9 +157,10 @@ export function decodeStreamError(data: Uint8Array): BaseError {
|
||||
}
|
||||
catch {}
|
||||
}
|
||||
const message = env.message !== undefined && env.message !== ''
|
||||
const rawMessage = env.message !== undefined && env.message !== ''
|
||||
? env.message
|
||||
: 'stream terminated by error event'
|
||||
const message = unwrapInvokeErrorMessage(rawMessage)
|
||||
const code = env.status !== undefined && env.status > 0 && env.status < 500
|
||||
? ErrorCode.Server4xxOther
|
||||
: ErrorCode.Server5xx
|
||||
@ -169,6 +170,25 @@ export function decodeStreamError(data: Uint8Array): BaseError {
|
||||
return err
|
||||
}
|
||||
|
||||
function unwrapInvokeErrorMessage(raw: string): string {
|
||||
if (!raw.startsWith('{'))
|
||||
return raw
|
||||
type InvokeErrorEnv = {
|
||||
error_type?: string
|
||||
args?: { description?: string }
|
||||
message?: string
|
||||
}
|
||||
try {
|
||||
const inner = JSON.parse(raw) as InvokeErrorEnv
|
||||
if (inner.error_type === undefined)
|
||||
return raw
|
||||
return inner.args?.description ?? inner.message ?? raw
|
||||
}
|
||||
catch {
|
||||
return raw
|
||||
}
|
||||
}
|
||||
|
||||
const SILENT_EVENTS = new Set([
|
||||
'node_retry',
|
||||
'iteration_started',
|
||||
|
||||
@ -1,5 +1,5 @@
|
||||
import type { CommandOutput } from './output.js'
|
||||
import type { ArgDefinition, OptionalArgValueType, FlagDefinition, ICommand, InferArgs, InferFlags } from './types.js'
|
||||
import type { ArgDefinition, FlagDefinition, ICommand, InferArgs, InferFlags, OptionalArgValueType } from './types.js'
|
||||
import { parseArgv } from './flags.js'
|
||||
|
||||
export type CommandConstructor = {
|
||||
|
||||
265
cli/src/framework/run.test.ts
Normal file
265
cli/src/framework/run.test.ts
Normal file
@ -0,0 +1,265 @@
|
||||
import type { CommandConstructor } from './command.js'
|
||||
import type { CommandTree } from './registry.js'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { BaseError, newError } from '../errors/base.js'
|
||||
import { ErrorCode, ExitCode } from '../errors/codes.js'
|
||||
import { Command } from './command.js'
|
||||
import { run, sniffOutputFormat } from './run.js'
|
||||
|
||||
describe('sniffOutputFormat', () => {
|
||||
it('returns empty for empty argv', () => {
|
||||
expect(sniffOutputFormat([])).toBe('')
|
||||
})
|
||||
|
||||
it('returns empty when no output flag present', () => {
|
||||
expect(sniffOutputFormat(['cmd'])).toBe('')
|
||||
expect(sniffOutputFormat(['cmd', 'pos', '--flag'])).toBe('')
|
||||
})
|
||||
|
||||
it('parses --output=value', () => {
|
||||
expect(sniffOutputFormat(['cmd', '--output=json'])).toBe('json')
|
||||
})
|
||||
|
||||
it('parses --output value (space form)', () => {
|
||||
expect(sniffOutputFormat(['cmd', '--output', 'json'])).toBe('json')
|
||||
})
|
||||
|
||||
it('parses -o value (space form)', () => {
|
||||
expect(sniffOutputFormat(['cmd', '-o', 'yaml'])).toBe('yaml')
|
||||
})
|
||||
|
||||
it('parses -o=value', () => {
|
||||
expect(sniffOutputFormat(['cmd', '-o=text'])).toBe('text')
|
||||
})
|
||||
|
||||
it('returns empty when next token after space-form is itself a flag', () => {
|
||||
expect(sniffOutputFormat(['cmd', '-o', '--other'])).toBe('')
|
||||
expect(sniffOutputFormat(['cmd', '--output', '--other'])).toBe('')
|
||||
})
|
||||
|
||||
it('stops at end-of-flags marker --', () => {
|
||||
expect(sniffOutputFormat(['cmd', '--', '-o', 'json'])).toBe('')
|
||||
expect(sniffOutputFormat(['cmd', '--', '--output=json'])).toBe('')
|
||||
})
|
||||
|
||||
it('first occurrence wins on duplicate flags', () => {
|
||||
expect(sniffOutputFormat(['cmd', '--output=json', '--output=yaml'])).toBe('json')
|
||||
})
|
||||
|
||||
it('does NOT support concatenated short form -o<val>', () => {
|
||||
expect(sniffOutputFormat(['cmd', '-ojson'])).toBe('')
|
||||
})
|
||||
})
|
||||
|
||||
type Captured = {
|
||||
stdout: string
|
||||
stderr: string
|
||||
exit: number | undefined
|
||||
}
|
||||
|
||||
async function captureRun(tree: CommandTree, argv: string[]): Promise<Captured> {
|
||||
const captured: Captured = { stdout: '', stderr: '', exit: undefined }
|
||||
const origStdout = process.stdout.write.bind(process.stdout)
|
||||
const origStderr = process.stderr.write.bind(process.stderr)
|
||||
const origExit = process.exit.bind(process)
|
||||
|
||||
process.stdout.write = ((chunk: string | Uint8Array) => {
|
||||
captured.stdout += typeof chunk === 'string' ? chunk : new TextDecoder().decode(chunk)
|
||||
return true
|
||||
}) as typeof process.stdout.write
|
||||
|
||||
process.stderr.write = ((chunk: string | Uint8Array) => {
|
||||
captured.stderr += typeof chunk === 'string' ? chunk : new TextDecoder().decode(chunk)
|
||||
return true
|
||||
}) as typeof process.stderr.write
|
||||
|
||||
process.exit = ((code?: number) => {
|
||||
captured.exit = code
|
||||
// do not throw; the catch block should `return` after exit
|
||||
}) as typeof process.exit
|
||||
|
||||
try {
|
||||
await run(tree, argv)
|
||||
}
|
||||
finally {
|
||||
process.stdout.write = origStdout
|
||||
process.stderr.write = origStderr
|
||||
process.exit = origExit
|
||||
}
|
||||
return captured
|
||||
}
|
||||
|
||||
function makeTree(cmd: CommandConstructor): CommandTree {
|
||||
return { cmd: { command: cmd, subcommands: {} } }
|
||||
}
|
||||
|
||||
describe('run() catch routing', () => {
|
||||
it('routes BaseError to human format with semantic exit code', async () => {
|
||||
class Throwing extends Command {
|
||||
async run(_argv: string[]) {
|
||||
throw new BaseError({
|
||||
code: ErrorCode.NotLoggedIn,
|
||||
message: 'not logged in',
|
||||
hint: 'run `difyctl auth login` to authenticate',
|
||||
})
|
||||
}
|
||||
}
|
||||
const result = await captureRun(makeTree(Throwing), ['cmd'])
|
||||
expect(result.stderr).toBe(
|
||||
'not_logged_in: not logged in\nhint: run `difyctl auth login` to authenticate\n',
|
||||
)
|
||||
expect(result.exit).toBe(ExitCode.Auth)
|
||||
expect(result.stdout).toBe('')
|
||||
})
|
||||
|
||||
it('routes BaseError to JSON envelope with --output=json', async () => {
|
||||
class Throwing extends Command {
|
||||
async run(_argv: string[]) {
|
||||
throw new BaseError({
|
||||
code: ErrorCode.NotLoggedIn,
|
||||
message: 'not logged in',
|
||||
hint: 'run `difyctl auth login` to authenticate',
|
||||
})
|
||||
}
|
||||
}
|
||||
const result = await captureRun(makeTree(Throwing), ['cmd', '--output=json'])
|
||||
expect(result.stderr).toBe(
|
||||
`${JSON.stringify({
|
||||
error: {
|
||||
code: 'not_logged_in',
|
||||
message: 'not logged in',
|
||||
hint: 'run `difyctl auth login` to authenticate',
|
||||
},
|
||||
})}\n`,
|
||||
)
|
||||
expect(result.exit).toBe(ExitCode.Auth)
|
||||
})
|
||||
|
||||
it('routes BaseError to JSON envelope with -o json (space form)', async () => {
|
||||
class Throwing extends Command {
|
||||
async run(_argv: string[]) {
|
||||
throw newError(ErrorCode.NotLoggedIn, 'not logged in')
|
||||
}
|
||||
}
|
||||
const result = await captureRun(makeTree(Throwing), ['cmd', '-o', 'json'])
|
||||
expect(result.stderr).toContain('"code":"not_logged_in"')
|
||||
expect(result.stderr.startsWith('{')).toBe(true)
|
||||
expect(result.exit).toBe(ExitCode.Auth)
|
||||
})
|
||||
|
||||
it('keeps human format when -- separates positional from --output', async () => {
|
||||
class Throwing extends Command {
|
||||
async run(_argv: string[]) {
|
||||
throw newError(ErrorCode.NotLoggedIn, 'not logged in')
|
||||
}
|
||||
}
|
||||
const result = await captureRun(makeTree(Throwing), ['cmd', '--', '--output=json'])
|
||||
expect(result.stderr.startsWith('not_logged_in:')).toBe(true)
|
||||
})
|
||||
|
||||
it('routes Usage error to exit code 2 with code prefix', async () => {
|
||||
class Throwing extends Command {
|
||||
async run(_argv: string[]) {
|
||||
throw newError(ErrorCode.UsageInvalidFlag, 'bad flag')
|
||||
}
|
||||
}
|
||||
const result = await captureRun(makeTree(Throwing), ['cmd'])
|
||||
expect(result.stderr).toBe('usage_invalid_flag: bad flag\n')
|
||||
expect(result.exit).toBe(ExitCode.Usage)
|
||||
})
|
||||
|
||||
it('routes Server5xx error with http_status line and generic exit', async () => {
|
||||
class Throwing extends Command {
|
||||
async run(_argv: string[]) {
|
||||
throw newError(ErrorCode.Server5xx, 'upstream boom').withHttpStatus(502)
|
||||
}
|
||||
}
|
||||
const result = await captureRun(makeTree(Throwing), ['cmd'])
|
||||
expect(result.stderr).toBe('server_5xx: upstream boom\nhttp_status: 502\n')
|
||||
expect(result.exit).toBe(ExitCode.Generic)
|
||||
})
|
||||
|
||||
it('renders request line and http_status when both are present', async () => {
|
||||
class Throwing extends Command {
|
||||
async run(_argv: string[]) {
|
||||
throw newError(ErrorCode.Server5xx, 'upstream boom')
|
||||
.withRequest('GET', 'https://api.dify.ai/v1/me')
|
||||
.withHttpStatus(502)
|
||||
}
|
||||
}
|
||||
const result = await captureRun(makeTree(Throwing), ['cmd'])
|
||||
expect(result.stderr).toBe(
|
||||
'server_5xx: upstream boom\nrequest: GET https://api.dify.ai/v1/me\nhttp_status: 502\n',
|
||||
)
|
||||
expect(result.exit).toBe(ExitCode.Generic)
|
||||
})
|
||||
|
||||
it('serializes method and url in JSON envelope', async () => {
|
||||
class Throwing extends Command {
|
||||
async run(_argv: string[]) {
|
||||
throw newError(ErrorCode.Server4xxOther, 'not found')
|
||||
.withRequest('GET', 'https://api.dify.ai/v1/apps/x')
|
||||
.withHttpStatus(404)
|
||||
}
|
||||
}
|
||||
const result = await captureRun(makeTree(Throwing), ['cmd', '--output=json'])
|
||||
const envelope = JSON.parse(result.stderr.trim())
|
||||
expect(envelope.error.method).toBe('GET')
|
||||
expect(envelope.error.url).toBe('https://api.dify.ai/v1/apps/x')
|
||||
expect(envelope.error.http_status).toBe(404)
|
||||
expect(envelope.error.code).toBe('server_4xx_other')
|
||||
expect(result.exit).toBe(ExitCode.Generic)
|
||||
})
|
||||
|
||||
it('falls through to generic Error branch and exits 1', async () => {
|
||||
class Throwing extends Command {
|
||||
async run(_argv: string[]) {
|
||||
throw new Error('oops')
|
||||
}
|
||||
}
|
||||
const result = await captureRun(makeTree(Throwing), ['cmd'])
|
||||
expect(result.stderr).toBe('oops\n')
|
||||
expect(result.exit).toBe(1)
|
||||
})
|
||||
|
||||
it('handles non-Error throw via String() coercion', async () => {
|
||||
class Throwing extends Command {
|
||||
async run(_argv: string[]) {
|
||||
// eslint-disable-next-line no-throw-literal
|
||||
throw 'plain string'
|
||||
}
|
||||
}
|
||||
const result = await captureRun(makeTree(Throwing), ['cmd'])
|
||||
expect(result.stderr).toBe('plain string\n')
|
||||
expect(result.exit).toBe(1)
|
||||
})
|
||||
|
||||
it('does not call process.exit when command runs successfully', async () => {
|
||||
class Ok extends Command {
|
||||
async run(_argv: string[]) {
|
||||
// returning void → run() does not write to stdout
|
||||
}
|
||||
}
|
||||
const result = await captureRun({ cmd: { command: Ok, subcommands: {} } }, ['cmd'])
|
||||
expect(result.stderr).toBe('')
|
||||
expect(result.exit).toBeUndefined()
|
||||
})
|
||||
|
||||
it('routes BaseError thrown from constructor through catch with JSON envelope', async () => {
|
||||
class CtorBang extends Command {
|
||||
constructor() {
|
||||
super()
|
||||
throw newError(ErrorCode.Unknown, 'ctor-bang')
|
||||
}
|
||||
|
||||
async run(_argv: string[]) {}
|
||||
}
|
||||
const result = await captureRun(
|
||||
{ cmd: { command: CtorBang, subcommands: {} } },
|
||||
['cmd', '--output=json'],
|
||||
)
|
||||
expect(result.stderr).toContain('"code":"unknown"')
|
||||
expect(result.stderr).toContain('"message":"ctor-bang"')
|
||||
expect(result.exit).toBe(ExitCode.Generic)
|
||||
})
|
||||
})
|
||||
@ -1,4 +1,6 @@
|
||||
import type { CommandTree } from './registry.js'
|
||||
import { BaseError } from '../errors/base.js'
|
||||
import { formatErrorForCli } from '../errors/format.js'
|
||||
import { formatHelp } from './help.js'
|
||||
import { stringifyOutput } from './output.js'
|
||||
import { findSuggestions, resolveCommand } from './registry.js'
|
||||
@ -46,17 +48,44 @@ export async function run(tree: CommandTree, argv: string[]): Promise<void> {
|
||||
process.stdout.write(stringifyOutput(output))
|
||||
}
|
||||
catch (err) {
|
||||
if (err instanceof BaseError) {
|
||||
const format = sniffOutputFormat(argv)
|
||||
process.stderr.write(`${formatErrorForCli(err, { format })}\n`)
|
||||
process.exit(err.exit())
|
||||
return
|
||||
}
|
||||
if (err instanceof Error) {
|
||||
process.stderr.write(`${err.message}\n`)
|
||||
process.exit(1)
|
||||
return
|
||||
}
|
||||
else {
|
||||
process.stderr.write(`${String(err)}\n`)
|
||||
}
|
||||
|
||||
process.stderr.write(`${String(err)}\n`)
|
||||
process.exit(1)
|
||||
}
|
||||
}
|
||||
|
||||
export function sniffOutputFormat(argv: readonly string[]): string {
|
||||
for (let i = 0; i < argv.length; i++) {
|
||||
const t = argv[i]
|
||||
if (t === undefined)
|
||||
continue
|
||||
if (t === '--')
|
||||
return ''
|
||||
|
||||
if (t === '--output' || t === '-o') {
|
||||
const next = argv[i + 1]
|
||||
if (next !== undefined && !next.startsWith('-'))
|
||||
return next
|
||||
continue
|
||||
}
|
||||
if (t.startsWith('--output='))
|
||||
return t.slice('--output='.length)
|
||||
if (t.startsWith('-o='))
|
||||
return t.slice('-o='.length)
|
||||
}
|
||||
return ''
|
||||
}
|
||||
|
||||
function printTopLevelHelp(tree: CommandTree): void {
|
||||
process.stdout.write('difyctl — Dify command-line interface\n\n')
|
||||
process.stdout.write('COMMANDS\n')
|
||||
|
||||
Reference in New Issue
Block a user