fix(openapi): harden formatter against malformed details and document bypass paths

Review follow-ups:
- finalize() now falls back to a minimal status-derived body instead of
  letting a ValidationError escape the framework error handler when an
  already-rewritten e.data carries malformed canonical details
- document that a pre-built e.response bypasses the body formatter
- note the promote-to-libs seam for transport-generic codes in the module
  docstring
- CLI: skip the loc prefix when a server error detail has an empty loc
This commit is contained in:
GareArc 2026-06-10 04:04:17 -07:00
parent 50573c78b9
commit 525d706bad
No known key found for this signature in database
5 changed files with 62 additions and 11 deletions

View File

@ -14,8 +14,13 @@ mandated by the OAuth spec)::
error-handler path funnels through one builder, and it also rewrites
``e.data`` because flask-restx ``Api.handle_error`` lets a pre-existing
``e.data`` override the registered handler's return value.
The transport-generic enum members, ``_CODE_BY_STATUS`` and the
``OpenApiError``/``OpenApiErrorFormatter`` bases are openapi-only today;
promote them to ``libs`` if a second surface adopts ``ErrorBody``.
"""
import logging
from enum import StrEnum
from typing import Any
@ -96,6 +101,8 @@ _CODE_BY_STATUS: dict[int, OpenApiErrorCode] = {
_GENERIC_500_MESSAGE = "Internal Server Error"
logger = logging.getLogger(__name__)
class OpenApiError(HTTPException):
"""Dedicated throwable for the /openapi/v1 surface.
@ -136,14 +143,24 @@ class OpenApiErrorFormatter:
exc_data = getattr(e, "data", None)
merged: dict[str, Any] = {**data, **exc_data} if isinstance(exc_data, dict) else dict(data)
body = ErrorBody(
code=self._resolve_code(e, status_code),
message=self._resolve_message(merged, status_code),
status=status_code,
hint=self._resolve_hint(e),
details=self._extract_details(e, merged),
)
wire = body.model_dump(mode="json", exclude_none=True)
# finalize runs inside the framework error handler: raising here would
# replace the response with an unformatted 500, so fall back instead
try:
body = ErrorBody(
code=self._resolve_code(e, status_code),
message=self._resolve_message(merged, status_code),
status=status_code,
hint=self._resolve_hint(e),
details=self._extract_details(e, merged),
)
wire = body.model_dump(mode="json", exclude_none=True)
except Exception:
logger.exception("error-body build failed; emitting fallback body")
wire = {
"code": str(_CODE_BY_STATUS.get(status_code, OpenApiErrorCode.UNKNOWN)),
"message": http_status_message(status_code) or "request failed",
"status": status_code,
}
# flask-restx Api.handle_error does `data = getattr(e, "data", default_data)`
# AFTER our handler returns, so a pre-existing e.data (flask_restx.abort,

View File

@ -32,7 +32,9 @@ def register_external_error_handlers(api: Api, body_formatter: ErrorBodyFormatte
def handle_http_exception(e: HTTPException):
got_request_exception.send(current_app, exception=e)
# If Werkzeug already prepared a Response, just use it.
# If Werkzeug already prepared a Response, just use it. This bypasses
# body_formatter entirely — surfaces with a formatter must not raise
# exceptions carrying a pre-built response.
if e.response is not None:
return e.response

View File

@ -126,6 +126,17 @@ class TestOpenApiErrorFormatter:
assert second == first
def test_malformed_canonical_details_falls_back_instead_of_raising(self, fmt):
# finalize runs inside the framework error handler; a ValidationError
# escaping it would replace the response with an unformatted 500
e = UnprocessableEntity()
e.data = {"message": "broken", "details": [{"bad": "shape"}]}
data = {"code": "unprocessable_entity", "message": "broken", "status": 422}
wire = fmt.finalize(e, data, 422)
assert wire == {"code": "invalid_param", "message": "Unprocessable Entity", "status": 422}
def test_base_http_exception_error_code_wins_over_status_map(self, fmt):
e = ProviderQuotaExceededError()
data = dict(e.data)

View File

@ -67,6 +67,25 @@ describe('formatErrorForCli — human', () => {
expect(formatErrorForCli(noServerHint, { isErrTTY: false })).toContain('run difyctl auth login')
})
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 })
expect(out).toContain('- body required (invalid)')
expect(out).not.toContain('- : body required')
})
it('renders request and http_status lines', () => {
const err = new HttpClientError({
code: ErrorCode.Server5xx,

View File

@ -50,8 +50,10 @@ function renderHuman(env: ErrorEnvelope, isErrTTY: boolean): string {
const server = e.server
const headerCode = server?.code ?? e.code
const lines: string[] = [`${headerCode}: ${e.message}`]
for (const d of server?.details ?? [])
lines.push(` - ${(d.loc ?? []).join('.')}: ${d.msg} (${d.type})`)
for (const d of server?.details ?? []) {
const loc = (d.loc ?? []).join('.')
lines.push(` - ${loc ? `${loc}: ` : ''}${d.msg} (${d.type})`)
}
const hint = server?.hint ?? e.hint
if (hint !== undefined && hint !== null)
lines.push(`${cs.magenta('hint:')} ${cs.cyan(hint)}`)