fix(api): map internal RetCode values to valid HTTP statuses in build_error_result (#18009)

### Summary

Fixes #17980.
This commit is contained in:
Charles
2026-08-10 11:30:11 +08:00
committed by GitHub
parent 3d41ebdded
commit 42373a8229
2 changed files with 148 additions and 1 deletions

View File

@@ -256,11 +256,39 @@ def get_json_result(code: RetCode = RetCode.SUCCESS, message="success", data=Non
return _safe_jsonify(response)
# Internal RetCode values below 200 collide with the HTTP 1xx (informational) range.
# h11 refuses to send a 1xx status as a final response, so Hypercorn drops the
# connection and the client receives an empty reply instead of the JSON error body.
# Map them to real HTTP statuses; the body keeps the original RetCode.
RET_CODE_TO_HTTP_STATUS = {
RetCode.EXCEPTION_ERROR: 500,
RetCode.ARGUMENT_ERROR: 400,
RetCode.DATA_ERROR: 400,
RetCode.OPERATING_ERROR: 400,
RetCode.CONNECTION_ERROR: 500,
RetCode.RUNNING: 500,
RetCode.PERMISSION_ERROR: 403,
# Dify's external knowledge API expects HTTP 403 for authorization failures.
RetCode.AUTHENTICATION_ERROR: 403,
}
def build_error_result(code=RetCode.FORBIDDEN, message="success"):
response = {"code": code, "message": message}
response = _safe_jsonify(response)
if hasattr(response, "status_code"):
response.status_code = code
ret_code = int(code)
http_status = RET_CODE_TO_HTTP_STATUS.get(ret_code)
# The status logs below carry the ret code only; `message` can hold user input.
if http_status is not None:
logging.debug("build_error_result: ret code %s mapped to HTTP %s", ret_code, http_status)
elif 200 <= ret_code <= 599:
http_status = ret_code
logging.debug("build_error_result: ret code %s used as HTTP status", ret_code)
else:
http_status = 500
logging.warning("build_error_result: unmapped ret code %s, falling back to HTTP 500", ret_code)
response.status_code = http_status
return response

View File

@@ -73,3 +73,122 @@ def test_get_data_openai_stream_delta_allows_reference_payload():
data["choices"][0]["delta"]["reference"] = {"chunks": []}
assert data["choices"][0]["delta"]["reference"] == {"chunks": []}
def _build_error_result_in_app_context(code):
import asyncio
from quart import Quart
app = Quart(__name__)
async def run():
async with app.app_context():
return api_utils.build_error_result(code=code, message="boom")
return asyncio.run(run())
def test_build_error_result_never_uses_1xx_http_status():
# RetCode.ARGUMENT_ERROR is 101. Sending it as the HTTP status makes h11
# reject the final response, so Hypercorn closes the connection and the
# client sees an empty reply instead of the JSON error (#17980).
from common.constants import RetCode
resp = _build_error_result_in_app_context(RetCode.ARGUMENT_ERROR)
assert resp.status_code == 400
# Plain ints must hit the same mapping (RetCode is an IntEnum).
assert _build_error_result_in_app_context(101).status_code == 400
def test_build_error_result_maps_internal_ret_codes():
from common.constants import RetCode
expected = {
RetCode.EXCEPTION_ERROR: 500,
RetCode.DATA_ERROR: 400,
RetCode.OPERATING_ERROR: 400,
RetCode.CONNECTION_ERROR: 500,
RetCode.RUNNING: 500,
RetCode.PERMISSION_ERROR: 403,
RetCode.AUTHENTICATION_ERROR: 403,
}
for code, http_status in expected.items():
resp = _build_error_result_in_app_context(code)
assert resp.status_code == http_status, code
assert resp.status_code >= 200
def test_build_error_result_falls_back_to_500_for_unmapped_sub_200_codes(caplog):
# NOT_EFFECTIVE is 10: below 200 and absent from RET_CODE_TO_HTTP_STATUS, so
# it must not reach the wire as an HTTP status of its own.
import logging
from common.constants import RetCode
caplog.set_level(logging.DEBUG)
resp = _build_error_result_in_app_context(RetCode.NOT_EFFECTIVE)
assert resp.status_code == 500
fallback_logs = [r for r in caplog.records if r.levelno == logging.WARNING and "unmapped ret code" in r.getMessage()]
assert len(fallback_logs) == 1, "the fallback branch must warn once about the unmapped ret code"
assert "unmapped ret code 10, falling back to HTTP 500" in fallback_logs[0].getMessage()
def test_build_error_result_status_logs_never_echo_the_message(caplog):
# `message` is caller-supplied and reaches the client verbatim; keep it out of
# the logs on every branch (mapped, passed through, and fallback).
import logging
from common.constants import RetCode
caplog.set_level(logging.DEBUG)
# (code, resolved status, level, branch marker) — one row per branch.
branches = [
(RetCode.ARGUMENT_ERROR, 400, logging.DEBUG, "mapped to HTTP"),
(RetCode.NOT_FOUND, 404, logging.DEBUG, "used as HTTP status"),
(RetCode.NOT_EFFECTIVE, 500, logging.WARNING, "falling back to HTTP"),
]
for code, http_status, level, marker in branches:
caplog.clear()
assert _build_error_result_in_app_context(code).status_code == http_status
status_logs = [r for r in caplog.records if "build_error_result:" in r.getMessage()]
assert len(status_logs) == 1, f"{code!r} must log exactly one status line, got {len(status_logs)}"
rendered = status_logs[0].getMessage()
assert status_logs[0].levelno == level, f"{code!r} logged at the wrong level: {rendered}"
assert marker in rendered, f"{code!r} took the wrong branch: {rendered}"
assert str(int(code)) in rendered, f"{code!r} must log its ret code: {rendered}"
assert "boom" not in rendered, "status logs must not echo the error message"
def test_build_error_result_default_code_is_a_valid_http_status():
# The signature default (FORBIDDEN) has to survive the mapping unchanged;
# callers that omit `code` must still get a sendable status.
import asyncio
from quart import Quart
app = Quart(__name__)
async def run():
async with app.app_context():
return api_utils.build_error_result(message="boom")
assert asyncio.run(run()).status_code == 403
def test_build_error_result_keeps_valid_http_codes_and_body():
import asyncio
from common.constants import RetCode
resp = _build_error_result_in_app_context(RetCode.NOT_FOUND)
assert resp.status_code == 404
body = asyncio.run(resp.get_json())
assert body == {"code": RetCode.NOT_FOUND, "message": "boom"}