From f3dde631dd34b8379f44685e65bb3b25301c32b2 Mon Sep 17 00:00:00 2001 From: GareArc Date: Wed, 10 Jun 2026 03:25:54 -0700 Subject: [PATCH] feat(cli): strict ErrorBody parse composed onto HttpClientError Replaces the tolerant WireFields/WireEnvelope parse in error-mapper with a strict zErrorBody.safeParse; non-conforming bodies yield no serverError and a generic message. The parsed body attaches whole as serverError?: ErrorBody on HttpClientError, with classification driven purely by HTTP status. --- cli/src/errors/base.ts | 11 +++++ cli/src/errors/format.ts | 2 + cli/src/http/error-mapper.test.ts | 75 +++++++++++++++++++++++++++++++ cli/src/http/error-mapper.ts | 60 ++++++++++++------------- cli/src/http/orpc.test.ts | 16 +++---- 5 files changed, 125 insertions(+), 39 deletions(-) create mode 100644 cli/src/http/error-mapper.test.ts diff --git a/cli/src/errors/base.ts b/cli/src/errors/base.ts index 0ad438a743..f1f9ca9853 100644 --- a/cli/src/errors/base.ts +++ b/cli/src/errors/base.ts @@ -1,3 +1,4 @@ +import type { ErrorBody } from '@dify/contracts/api/openapi/types.gen' import type { ErrorCodeValue, ExitCodeValue } from './codes' import type { ErrorEnvelope, PrintableError } from './format' import { ErrorCode, exitFor } from './codes' @@ -83,6 +84,7 @@ type HttpClientErrorOptions = BaseErrorOptions & { readonly method?: string readonly url?: string readonly rawResponse?: string + readonly serverError?: ErrorBody } export class HttpClientError extends BaseError { @@ -90,6 +92,7 @@ export class HttpClientError extends BaseError { readonly method?: string readonly url?: string readonly rawResponse?: string + readonly serverError?: ErrorBody constructor(opts: HttpClientErrorOptions) { super(opts) @@ -97,6 +100,7 @@ export class HttpClientError extends BaseError { this.method = opts.method this.url = opts.url this.rawResponse = opts.rawResponse + this.serverError = opts.serverError } override toEnvelope(): ErrorEnvelope { @@ -109,6 +113,8 @@ export class HttpClientError extends BaseError { envelope.error.url = this.url if (this.rawResponse !== undefined) envelope.error.raw_response = this.rawResponse + if (this.serverError !== undefined) + envelope.error.server = this.serverError return envelope } @@ -119,6 +125,7 @@ export class HttpClientError extends BaseError { method: this.method, url: this.url, rawResponse: this.rawResponse, + serverError: this.serverError, } } @@ -145,4 +152,8 @@ export class HttpClientError extends BaseError { } return new HttpClientError({ ...this.snapshot(), rawResponse }) } + + withServerError(serverError: ErrorBody): HttpClientError { + return new HttpClientError({ ...this.snapshot(), serverError }) + } } diff --git a/cli/src/errors/format.ts b/cli/src/errors/format.ts index b8c3fe6cab..97bc527e7d 100644 --- a/cli/src/errors/format.ts +++ b/cli/src/errors/format.ts @@ -1,3 +1,4 @@ +import type { ErrorBody } from '@dify/contracts/api/openapi/types.gen' import { isVerbose } from '@/framework/context' import { redactBearer } from '@/http/sanitize' import { colorEnabled, colorScheme } from '@/sys/io/color' @@ -16,6 +17,7 @@ export type ErrorEnvelope = { method?: string url?: string raw_response?: string + server?: ErrorBody } } diff --git a/cli/src/http/error-mapper.test.ts b/cli/src/http/error-mapper.test.ts new file mode 100644 index 0000000000..33ec34dd2c --- /dev/null +++ b/cli/src/http/error-mapper.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it } from 'vitest' +import { isHttpClientError } from '@/errors/base' +import { ErrorCode } from '@/errors/codes' +import { classifyResponse } from './error-mapper' + +function res(status: number, body: unknown): Response { + return new Response(typeof body === 'string' ? body : JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }) +} + +const req = new Request('https://dify.test/openapi/v1/apps') + +describe('classifyResponse — canonical ErrorBody', () => { + it('attaches the parsed body whole as serverError', async () => { + const body = { + code: 'invalid_param', + message: 'Request validation failed', + status: 422, + hint: 'check the page parameter', + details: [{ type: 'int_parsing', loc: ['page'], msg: 'must be >= 1' }], + } + + const err = await classifyResponse(req, res(422, body)) + + expect(isHttpClientError(err)).toBe(true) + if (!isHttpClientError(err)) + return + expect(err.serverError).toEqual(body) + expect(err.message).toBe('Request validation failed') + expect(err.code).toBe(ErrorCode.Server4xxOther) + }) + + it('401 classifies by status as AuthExpired with CLI login hint', async () => { + const err = await classifyResponse(req, res(401, { + code: 'unauthorized', + message: 'session expired or revoked', + status: 401, + })) + + expect(err.code).toBe(ErrorCode.AuthExpired) + expect(err.hint).toBe('run \'difyctl auth login\' to sign in again') + }) + + it('unknown future server code is data, not behavior — status bucket decides', async () => { + const err = await classifyResponse(req, res(409, { + code: 'some_future_code', + message: 'nope', + status: 409, + })) + + expect(err.code).toBe(ErrorCode.Server4xxOther) + if (isHttpClientError(err)) + expect(err.serverError?.code).toBe('some_future_code') + }) +}) + +describe('classifyResponse — non-conforming bodies (no fallback by design)', () => { + it('non-JSON body yields no serverError, classification by status', async () => { + const err = await classifyResponse(req, res(502, 'bad gateway')) + + expect(err.code).toBe(ErrorCode.Server5xx) + if (isHttpClientError(err)) + expect(err.serverError).toBeUndefined() + }) + + it('RFC 8628 string error field yields no serverError and a generic message', async () => { + const err = await classifyResponse(req, res(400, { error: 'slow_down' })) + + expect(err.message).toBe('request failed (HTTP 400)') + if (isHttpClientError(err)) + expect(err.serverError).toBeUndefined() + }) +}) diff --git a/cli/src/http/error-mapper.ts b/cli/src/http/error-mapper.ts index cb0d03c206..86e4c65fa4 100644 --- a/cli/src/http/error-mapper.ts +++ b/cli/src/http/error-mapper.ts @@ -1,70 +1,70 @@ +import type { ErrorBody } from '@dify/contracts/api/openapi/types.gen' +import { zErrorBody } from '@dify/contracts/api/openapi/zod.gen' import { BaseError, HttpClientError, newError } from '@/errors/base' import { ErrorCode } from '@/errors/codes' import { redactBearer } from './sanitize' -type WireFields = { - code?: string - message?: string - hint?: string -} +const AUTH_EXPIRED_MESSAGE = 'session expired or revoked' +const AUTH_LOGIN_HINT = 'run \'difyctl auth login\' to sign in again' -type WireEnvelope = WireFields & { - error?: WireFields -} - -async function readBody(response: Response): Promise<{ raw: string, parsed?: WireEnvelope }> { - let raw = '' - try { - raw = await response.text() - } - catch { - return { raw: '' } - } +function parseServerError(raw: string): ErrorBody | undefined { if (raw === '') - return { raw } + return undefined + let parsed: unknown try { - return { raw, parsed: JSON.parse(raw) as WireEnvelope } + parsed = JSON.parse(raw) } catch { - return { raw } + return undefined } + const result = zErrorBody.safeParse(parsed) + return result.success ? result.data : undefined } export async function classifyResponse(request: Request, response: Response): Promise { - const { parsed, raw } = await readBody(response.clone()) - const wire: WireFields = parsed?.error ?? parsed ?? {} + let raw = '' + try { + raw = await response.clone().text() + } + catch { + // ignore read errors; raw stays '' + } + + const serverError = parseServerError(raw) const status = response.status const url = redactBearer(response.url || request.url) const method = request.method if (status === 401) { - return HttpClientError.from(newError( + const base = HttpClientError.from(newError( ErrorCode.AuthExpired, - wire.message ?? 'session expired or revoked', + serverError?.message ?? AUTH_EXPIRED_MESSAGE, )) - .withHint(wire.hint ?? 'run \'difyctl auth login\' to sign in again') + .withHint(AUTH_LOGIN_HINT) .withHttpStatus(status) .withRequest(method, url) + return serverError !== undefined ? base.withServerError(serverError) : base } if (status >= 500) { - return HttpClientError.from(newError( + const base = HttpClientError.from(newError( ErrorCode.Server5xx, - wire.message ?? `server error (HTTP ${status})`, + serverError?.message ?? `server error (HTTP ${status})`, )) .withHttpStatus(status) .withRequest(method, url) .withRawResponse(raw) + return serverError !== undefined ? base.withServerError(serverError) : base } - const err = HttpClientError.from(newError( + const base = HttpClientError.from(newError( ErrorCode.Server4xxOther, - wire.message ?? `request failed (HTTP ${status})`, + serverError?.message ?? `request failed (HTTP ${status})`, )) .withHttpStatus(status) .withRequest(method, url) .withRawResponse(raw) - return wire.hint !== undefined ? err.withHint(wire.hint) : err + return serverError !== undefined ? base.withServerError(serverError) : base } export function classifyTransportError(err: unknown): BaseError { diff --git a/cli/src/http/orpc.test.ts b/cli/src/http/orpc.test.ts index 05e9a8405f..85023792ee 100644 --- a/cli/src/http/orpc.test.ts +++ b/cli/src/http/orpc.test.ts @@ -33,8 +33,8 @@ describe('createOpenApiClient error mapping', () => { await stub?.stop() }) - it('recovers Dify message + hint from a top-level 4xx envelope', async () => { - stub = await startStubServer(cap => jsonResponder(403, { message: 'no access', hint: 'ask an admin' }, cap)) + it('recovers Dify message from a canonical ErrorBody 4xx response', async () => { + stub = await startStubServer(cap => jsonResponder(403, { code: 'access_denied', message: 'no access', status: 403 }, cap)) const orpc = orpcClient(stub.url) const caught = await catchErr(() => orpc.account.get()) @@ -44,7 +44,6 @@ describe('createOpenApiClient error mapping', () => { expect(caught.code).toBe(ErrorCode.Server4xxOther) expect(caught.httpStatus).toBe(403) expect(caught.message).toBe('no access') - expect(caught.hint).toBe('ask an admin') // Parity with the transport path: the migrated endpoint's error keeps the request // method/url and the raw body, so formatted errors still print the `request:` line // and the raw-response dump (not just message/hint). @@ -54,8 +53,8 @@ describe('createOpenApiClient error mapping', () => { } }) - it('recovers from a nested { error: { message, hint } } envelope and keeps the auth code on 401', async () => { - stub = await startStubServer(cap => jsonResponder(401, { error: { message: 'expired', hint: 'relogin' } }, cap)) + it('reads server message from canonical ErrorBody on 401 and keeps the auth code', async () => { + stub = await startStubServer(cap => jsonResponder(401, { code: 'unauthorized', message: 'expired', status: 401 }, cap)) const orpc = orpcClient(stub.url) const caught = await catchErr(() => orpc.account.get()) @@ -65,11 +64,10 @@ describe('createOpenApiClient error mapping', () => { expect(caught.code).toBe(ErrorCode.AuthExpired) expect(caught.httpStatus).toBe(401) expect(caught.message).toBe('expired') - expect(caught.hint).toBe('relogin') } }) - it('falls back to the default auth-login hint when the body carries none', async () => { + it('uses CLI default auth-login hint for non-conforming 401 body', async () => { stub = await startStubServer(cap => jsonResponder(401, { error: 'expired' }, cap)) const orpc = orpcClient(stub.url) @@ -82,8 +80,8 @@ describe('createOpenApiClient error mapping', () => { } }) - it('maps 5xx to Server5xx', async () => { - stub = await startStubServer(cap => jsonResponder(503, { message: 'down for maintenance' }, cap)) + it('maps 5xx to Server5xx with message from canonical ErrorBody', async () => { + stub = await startStubServer(cap => jsonResponder(503, { code: 'service_unavailable', message: 'down for maintenance', status: 503 }, cap)) const orpc = orpcClient(stub.url) const caught = await catchErr(() => orpc.account.get())