test: dedupe error-path test helpers in cli and api

This commit is contained in:
GareArc 2026-06-10 19:06:59 -07:00
parent 673eb6bff9
commit 5b75aae20d
No known key found for this signature in database
4 changed files with 75 additions and 105 deletions

View File

@ -42,6 +42,11 @@ from controllers.service_api.app.error import (
from controllers.web.error import InvokeRateLimitError as InvokeRateLimitHttpError
@pytest.fixture
def fmt() -> OpenApiErrorFormatter:
return OpenApiErrorFormatter()
class TestErrorBodyModel:
def test_minimal_body_serializes_without_optional_fields(self):
body = ErrorBody(code=OpenApiErrorCode.NOT_FOUND, message="app not found", status=404)
@ -73,10 +78,6 @@ class TestErrorBodyModel:
class TestOpenApiErrorFormatter:
@pytest.fixture
def fmt(self):
return OpenApiErrorFormatter()
def test_plain_werkzeug_exception_maps_code_from_status(self, fmt):
e = NotFound("app not found")
data = {"code": "not_found", "message": "app not found", "status": 404}
@ -222,10 +223,6 @@ class TestOpenApiErrorFormatter:
class TestQuotaExceptions:
@pytest.fixture
def fmt(self):
return OpenApiErrorFormatter()
@pytest.mark.parametrize("exc_class", [MemberLimitExceeded, MemberLicenseExceeded])
def test_quota_exception_carries_declared_code_and_message(self, fmt, exc_class):
# Single source: assertions read the class attributes, no re-typed strings.
@ -329,8 +326,7 @@ class TestErrorMatrix:
ERROR_MATRIX,
ids=lambda v: type(v).__name__ if isinstance(v, Exception) else str(v),
)
def test_every_known_error_path_yields_canonical_code(self, exc, status, expected_code):
fmt = OpenApiErrorFormatter()
def test_every_known_error_path_yields_canonical_code(self, fmt, exc, status, expected_code):
data = dict(getattr(exc, "data", None) or {"message": str(exc), "status": status})
wire = fmt.finalize(exc, data, status)

View File

@ -1,29 +1,41 @@
import type { ErrorBody } from '@dify/contracts/api/openapi/types.gen'
import { describe, expect, it } from 'vitest'
import { HttpClientError } from './base'
import { ErrorCode } from './codes'
import { formatErrorForCli } from './format'
function validationError(): HttpClientError {
type ValidationErrorOverrides = {
readonly cliHint?: string
readonly serverHint?: string
readonly details?: ErrorBody['details']
}
function validationError(overrides: ValidationErrorOverrides = {}): HttpClientError {
const details
= overrides.details
?? [
{ type: 'int_parsing', loc: ['page'], msg: 'must be >= 1' },
{ type: 'missing', loc: ['inputs', 'query'], msg: 'field required' },
]
return new HttpClientError({
code: ErrorCode.Server4xxOther,
message: 'Request validation failed',
httpStatus: 422,
hint: overrides.cliHint,
serverError: {
code: 'invalid_param',
message: 'Request validation failed',
status: 422,
hint: 'check the page parameter',
details: [
{ type: 'int_parsing', loc: ['page'], msg: 'must be >= 1' },
{ type: 'missing', loc: ['inputs', 'query'], msg: 'field required' },
],
hint: overrides.serverHint,
details,
},
})
}
describe('formatErrorForCli — human', () => {
it('prints server code, message, and details without verbose', () => {
const out = formatErrorForCli(validationError(), { isErrTTY: false })
const out = formatErrorForCli(validationError({ serverHint: 'check the page parameter' }), { isErrTTY: false })
expect(out).toContain('invalid_param: Request validation failed')
expect(out).toContain('- page: must be >= 1 (int_parsing)')
@ -41,20 +53,7 @@ describe('formatErrorForCli — human', () => {
})
it('server hint wins over cli hint; cli hint fills when server sent none', () => {
// validationError has server hint "check the page parameter"; cli hint is ignored
const withCliHint = new HttpClientError({
code: ErrorCode.Server4xxOther,
message: 'Request validation failed',
httpStatus: 422,
hint: 'cli fallback hint',
serverError: {
code: 'invalid_param',
message: 'Request validation failed',
status: 422,
hint: 'check the page parameter',
details: [],
},
})
const withCliHint = validationError({ cliHint: 'cli fallback hint', serverHint: 'check the page parameter', details: [] })
expect(formatErrorForCli(withCliHint, { isErrTTY: false })).toContain('check the page parameter')
expect(formatErrorForCli(withCliHint, { isErrTTY: false })).not.toContain('cli fallback hint')
@ -68,19 +67,10 @@ describe('formatErrorForCli — human', () => {
})
it('omits the loc prefix when a detail has no loc', () => {
const err = new HttpClientError({
code: ErrorCode.Server4xxOther,
message: 'Request validation failed',
httpStatus: 422,
serverError: {
code: 'invalid_param',
message: 'Request validation failed',
status: 422,
details: [{ type: 'invalid', loc: [], msg: 'body required' }],
},
})
const out = formatErrorForCli(err, { isErrTTY: false })
const out = formatErrorForCli(
validationError({ details: [{ type: 'invalid', loc: [], msg: 'body required' }] }),
{ isErrTTY: false },
)
expect(out).toContain('- body required (invalid)')
expect(out).not.toContain('- : body required')

View File

@ -1,5 +1,5 @@
import type { HttpClientError } from '@/errors/base'
import { describe, expect, it } from 'vitest'
import { isHttpClientError } from '@/errors/base'
import { ErrorCode } from '@/errors/codes'
import { classifyResponse } from './error-mapper'
@ -12,6 +12,10 @@ function res(status: number, body: unknown): Response {
const req = new Request('https://dify.test/openapi/v1/apps')
function classified(status: number, body: unknown): Promise<HttpClientError> {
return classifyResponse(req, res(status, body))
}
describe('classifyResponse — canonical ErrorBody', () => {
it('attaches the parsed body whole as serverError', async () => {
const body = {
@ -22,54 +26,48 @@ describe('classifyResponse — canonical ErrorBody', () => {
details: [{ type: 'int_parsing', loc: ['page'], msg: 'must be >= 1' }],
}
const err = await classifyResponse(req, res(422, body))
const err = await classified(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, {
const err = await classified(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, {
const err = await classified(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')
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, '<html>bad gateway</html>'))
const err = await classified(502, '<html>bad gateway</html>')
expect(err.code).toBe(ErrorCode.Server5xx)
if (isHttpClientError(err))
expect(err.serverError).toBeUndefined()
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' }))
const err = await classified(400, { error: 'slow_down' })
expect(err.message).toBe('request failed (HTTP 400)')
if (isHttpClientError(err))
expect(err.serverError).toBeUndefined()
expect(err.serverError).toBeUndefined()
})
})

View File

@ -1,4 +1,5 @@
import type { StubServer } from '@test/fixtures/stub-server'
import type { HttpClientError } from '@/errors/base'
import { jsonResponder, startStubServer } from '@test/fixtures/stub-server'
import { afterEach, describe, expect, it } from 'vitest'
import { isHttpClientError } from '@/errors/base'
@ -33,64 +34,49 @@ describe('createOpenApiClient error mapping', () => {
await stub?.stop()
})
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))
async function classifiedError(status: number, body: unknown): Promise<HttpClientError> {
stub = await startStubServer(cap => jsonResponder(status, body, cap))
const orpc = orpcClient(stub.url)
const caught = await catchErr(() => orpc.account.get())
if (!isHttpClientError(caught))
throw new Error(`expected HttpClientError, got: ${String(caught)}`)
return caught
}
expect(isHttpClientError(caught)).toBe(true)
if (isHttpClientError(caught)) {
expect(caught.code).toBe(ErrorCode.Server4xxOther)
expect(caught.httpStatus).toBe(403)
expect(caught.message).toBe('no access')
// 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).
expect(caught.method).toBe('GET')
expect(caught.url).toContain('/account')
expect(caught.rawResponse).toContain('no access')
}
it('recovers Dify message from a canonical ErrorBody 4xx response', async () => {
const caught = await classifiedError(403, { code: 'access_denied', message: 'no access', status: 403 })
expect(caught.code).toBe(ErrorCode.Server4xxOther)
expect(caught.httpStatus).toBe(403)
expect(caught.message).toBe('no access')
// 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).
expect(caught.method).toBe('GET')
expect(caught.url).toContain('/account')
expect(caught.rawResponse).toContain('no access')
})
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 classifiedError(401, { code: 'unauthorized', message: 'expired', status: 401 })
const caught = await catchErr(() => orpc.account.get())
expect(isHttpClientError(caught)).toBe(true)
if (isHttpClientError(caught)) {
expect(caught.code).toBe(ErrorCode.AuthExpired)
expect(caught.httpStatus).toBe(401)
expect(caught.message).toBe('expired')
}
expect(caught.code).toBe(ErrorCode.AuthExpired)
expect(caught.httpStatus).toBe(401)
expect(caught.message).toBe('expired')
})
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)
const caught = await classifiedError(401, { error: 'expired' })
const caught = await catchErr(() => orpc.account.get())
expect(isHttpClientError(caught)).toBe(true)
if (isHttpClientError(caught)) {
expect(caught.code).toBe(ErrorCode.AuthExpired)
expect(caught.hint).toContain('difyctl auth login')
}
expect(caught.code).toBe(ErrorCode.AuthExpired)
expect(caught.hint).toContain('difyctl auth login')
})
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 classifiedError(503, { code: 'service_unavailable', message: 'down for maintenance', status: 503 })
const caught = await catchErr(() => orpc.account.get())
expect(isHttpClientError(caught)).toBe(true)
if (isHttpClientError(caught)) {
expect(caught.code).toBe(ErrorCode.Server5xx)
expect(caught.httpStatus).toBe(503)
expect(caught.message).toBe('down for maintenance')
}
expect(caught.code).toBe(ErrorCode.Server5xx)
expect(caught.httpStatus).toBe(503)
expect(caught.message).toBe('down for maintenance')
})
})