diff --git a/cli/src/api/app-meta.test.ts b/cli/src/api/app-meta.test.ts index ab7ab8a6ae1..a8ef958ce2f 100644 --- a/cli/src/api/app-meta.test.ts +++ b/cli/src/api/app-meta.test.ts @@ -1,13 +1,14 @@ import type { DifyMock } from '@test/fixtures/dify-mock/server' -import { mkdtemp, rm } from 'node:fs/promises' +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { startMock } from '@test/fixtures/dify-mock/server' import { testHttpClient } from '@test/fixtures/http-client' +import yaml from 'js-yaml' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { loadAppInfoCache } from '@/cache/app-info' import { ENV_CACHE_DIR } from '@/store/dir' -import { CACHE_APP_INFO, getCache } from '@/store/manager' +import { CACHE_APP_INFO, cachePath, getCache } from '@/store/manager' import { FieldInfo, FieldParameters } from '@/types/app-meta' import { AppMetaClient } from './app-meta.js' import { AppsClient } from './apps.js' @@ -97,4 +98,40 @@ describe('AppMetaClient', () => { await client.get('app-1', [FieldInfo]) expect(spy).toHaveBeenCalledTimes(2) }) + + it('corrupt cache entry refetches; valid sibling stays cached; no throw', async () => { + const path = cachePath(dir, CACHE_APP_INFO) + const apps = new AppsClient(testHttpClient(mock.url, 'dfoa_test')) + + // Seed a real, production-serialized entry by fetching app-1 once (this + // calls cache.set → serialize, so we never hand-write the on-disk shape). + const seed = await loadAppInfoCache({ store: getCache(CACHE_APP_INFO) }) + await new AppMetaClient({ apps, host: mock.url, cache: seed }).get('app-1', [FieldInfo]) + + // Reuse that serialized entry as a valid sibling; corrupt the app-1 slot. + const file = yaml.load(await readFile(path, 'utf8')) as { entries: Record } + const validEntry = file.entries[`${mock.url}::app-1`] + await writeFile( + path, + yaml.dump({ entries: { + [`${mock.url}::app-1`]: 'corrupted-string', + [`${mock.url}::sibling`]: validEntry, + } }), + 'utf8', + ) + + // Reload: app-1 dropped, sibling kept. + const cache = await loadAppInfoCache({ store: getCache(CACHE_APP_INFO) }) + const spy = vi.spyOn(apps, 'describe') + const client = new AppMetaClient({ apps, host: mock.url, cache }) + + // app-1 corrupt → dropped → miss → refetched from the mock + const a = await client.get('app-1', [FieldInfo]) + expect(a.info?.id).toBe('app-1') + expect(spy).toHaveBeenCalledTimes(1) + + // sibling is the real serialized entry → served from cache, no network + await client.get('sibling', [FieldInfo]) + expect(spy).toHaveBeenCalledTimes(1) + }) }) diff --git a/cli/src/cache/app-info.test.ts b/cli/src/cache/app-info.test.ts index ae6f15f249d..59d8e762020 100644 --- a/cli/src/cache/app-info.test.ts +++ b/cli/src/cache/app-info.test.ts @@ -1,5 +1,5 @@ import type { AppMeta } from '@/types/app-meta' -import { mkdtemp, readFile, rm } from 'node:fs/promises' +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import yaml from 'js-yaml' @@ -14,10 +14,10 @@ function appInfoPath(dir: string): string { return cachePath(dir, CACHE_APP_INFO) } -function metaInfoOnly(): AppMeta { +function metaInfoOnly(id = 'app-1'): AppMeta { return { info: { - id: 'app-1', + id, name: 'Greeter', description: '', mode: 'chat', @@ -101,12 +101,32 @@ describe('app-info disk cache', () => { }) it('corrupt cache file is treated as empty', async () => { - const { writeFile } = await import('node:fs/promises') await writeFile(appInfoPath(dir), ': : not valid yaml', 'utf8') const c = await loadAppInfoCache({ store: getCache(CACHE_APP_INFO) }) expect(c.get('h', 'app-1')).toBeUndefined() }) + it('drops a corrupt single entry but keeps valid siblings', async () => { + // Seed a real serialized entry via set() — no hand-authored on-disk shape. + const seed = await loadAppInfoCache({ store: getCache(CACHE_APP_INFO) }) + await seed.set('h', 'app-2', metaInfoOnly('app-2')) + + // Inject a corrupt sibling alongside the real one. + const file = yaml.load(await readFile(appInfoPath(dir), 'utf8')) as { entries: Record } + file.entries['h::app-1'] = 'corrupted-string-not-object' + await writeFile(appInfoPath(dir), yaml.dump(file), 'utf8') + + const c = await loadAppInfoCache({ store: getCache(CACHE_APP_INFO) }) + expect(c.get('h', 'app-1')).toBeUndefined() + expect(c.get('h', 'app-2')?.meta.info?.id).toBe('app-2') + }) + + it('treats a non-object entries map as empty', async () => { + await writeFile(appInfoPath(dir), yaml.dump({ entries: 'not-an-object' }), 'utf8') + const c = await loadAppInfoCache({ store: getCache(CACHE_APP_INFO) }) + expect(c.get('h', 'app-1')).toBeUndefined() + }) + it('updates same key in place (no growth)', async () => { const c = await loadAppInfoCache({ store: getCache(CACHE_APP_INFO) }) await c.set('h', 'app-1', metaInfoOnly()) diff --git a/cli/src/cache/app-info.ts b/cli/src/cache/app-info.ts index 052de019fee..266a880efce 100644 --- a/cli/src/cache/app-info.ts +++ b/cli/src/cache/app-info.ts @@ -74,8 +74,18 @@ async function readEntries(store: Store): Promise { expect(result.exit).toBe(ExitCode.Generic) }) - it('falls through to generic Error branch and exits 1', async () => { + it('routes non-BaseError to JSON envelope with -o json (exit 1)', async () => { + class Throwing extends Command { + async run(_argv: string[]) { + throw new Error('boom') + } + } + const result = await captureRun(makeTree(Throwing), ['cmd', '-o', 'json']) + expect(result.stderr).toBe(`${JSON.stringify({ error: { code: 'unknown', message: 'boom' } })}\n`) + expect(result.exit).toBe(ExitCode.Generic) + expect(result.stdout).toBe('') + }) + + it('wraps a generic Error into the human unknown form 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) + expect(result.stderr).toBe('unknown: oops\n') + expect(result.exit).toBe(ExitCode.Generic) }) - it('handles non-Error throw via String() coercion', async () => { + it('wraps a non-Error throw via String() coercion into unknown form', async () => { class Throwing extends Command { async run(_argv: string[]) { // eslint-disable-next-line no-throw-literal @@ -231,8 +243,52 @@ describe('run() catch routing', () => { } } const result = await captureRun(makeTree(Throwing), ['cmd']) - expect(result.stderr).toBe('plain string\n') - expect(result.exit).toBe(1) + expect(result.stderr).toBe('unknown: plain string\n') + expect(result.exit).toBe(ExitCode.Generic) + }) + + it('exits 0 on EPIPE without writing an error envelope', async () => { + class Throwing extends Command { + async run(_argv: string[]) { + throw Object.assign(new Error('broken pipe'), { code: 'EPIPE' }) + } + } + // process.exit is typed `never`; stub it to halt (throw) like the real call, + // so the EPIPE early-exit doesn't fall through to the envelope path. + let exitCode: number | undefined + let stderr = '' + const origExit = process.exit.bind(process) + const origStderr = process.stderr.write.bind(process.stderr) + process.exit = ((code?: number) => { + exitCode = code + throw new Error('__exit__') + }) as typeof process.exit + process.stderr.write = ((chunk: string | Uint8Array) => { + stderr += typeof chunk === 'string' ? chunk : new TextDecoder().decode(chunk) + return true + }) as typeof process.stderr.write + try { + await run(makeTree(Throwing), ['cmd', '-o', 'json']) + } + catch (e) { + expect((e as Error).message).toBe('__exit__') + } + finally { + process.exit = origExit + process.stderr.write = origStderr + } + expect(exitCode).toBe(0) + expect(stderr).toBe('') + }) + + it('preserves RateLimited semantic exit code through the collapsed catch', async () => { + class Throwing extends Command { + async run(_argv: string[]) { + throw newError(ErrorCode.RateLimited, 'slow down') + } + } + const result = await captureRun(makeTree(Throwing), ['cmd']) + expect(result.exit).toBe(ExitCode.RateLimited) }) it('does not call process.exit when command runs successfully', async () => { diff --git a/cli/src/framework/run.ts b/cli/src/framework/run.ts index 5682a9e1887..9bbd86aa5e2 100644 --- a/cli/src/framework/run.ts +++ b/cli/src/framework/run.ts @@ -1,5 +1,5 @@ import type { CommandTree } from './registry' -import { BaseError } from '@/errors/base' +import { BaseError, unknownError } from '@/errors/base' import { formatErrorForCli } from '@/errors/format' import { findTopic } from '@/help/topics' import { formatCommandList, formatHelp, formatTopic, formatTopLevelHelp } from './help' @@ -106,19 +106,12 @@ export async function run(tree: CommandTree, argv: string[]): Promise { catch (err) { if ((err as NodeJS.ErrnoException).code === 'EPIPE') process.exit(0) - if (err instanceof BaseError) { - const format = sniffOutputFormat(argv) - process.stderr.write(`${formatErrorForCli(err, { format, isErrTTY: process.stderr.isTTY })}\n`) - process.exit(err.exit()) - return - } - if (err instanceof Error) { - process.stderr.write(`${err.message}\n`) - process.exit(1) - return - } - process.stderr.write(`${String(err)}\n`) - process.exit(1) + const e = err instanceof BaseError + ? err + : unknownError(err instanceof Error ? err.message : String(err), err) + const format = sniffOutputFormat(argv) + process.stderr.write(`${formatErrorForCli(e, { format, isErrTTY: process.stderr.isTTY })}\n`) + process.exit(e.exit()) } }