feat(etl-uvicorn): carry failure_category through invoke and precheck responses - #77
feat(etl-uvicorn): carry failure_category through invoke and precheck responses#77CyMule wants to merge 9 commits into
Conversation
Guard failure_category and status_code pickup against raising descriptors and non-int values, serialize non-string HTTPException details, and repair the inverted single-parameter validation in check_precheck_func.
…echeck path Share one guarded attribute reader between failure_category_of and status_code_of, apply status_code_of at the UnstructuredIngestError site it missed, compute the function signature once per request, and emit the missing-usage-parameter warning once at wrap time instead of on every request. Reuse existing test scaffolding instead of duplicating it.
Clamp status_code_of to the HTTP status range (0 regressed to being served verbatim instead of falling back to 500), read status_code through the guarded accessor in the UnstructuredIngestError log line, and resolve string/postponed annotations before validating precheck signatures.
run_in_executor does not copy contextvars, so OpenTelemetry context was lost crossing into the worker thread and wide events adopted inside synchronous invoke/precheck functions found no active span. asyncio.to_thread copies the calling context.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Shadow auto-approve: would require human review. Adds a new field to response models, changing the public API contract; feature expansion for plugin precheck failure reporting requires product/contract decision.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would require human review. Adds failure_category to public response models (API shape change) and switches invoke executor to to_thread for context propagation, an operational tradeoff. Requires human review for API compatibility and runtime behavior.
Re-trigger cubic
|
why "failure_category" need add to "InvokeResponse" ? |
| status_code=getattr(invoke_error, "status_code", None) | ||
| or status.HTTP_500_INTERNAL_SERVER_ERROR, | ||
| status_code=status_code_of(invoke_error), | ||
| status_code_text=f"[{invoke_error.__class__.__name__}] {invoke_error}", |
There was a problem hiding this comment.
suggest to have : One helper, applied consistently:
def _safe_str(error: BaseException) -> str:
"""str() on a plugin-supplied error can itself raise; never let that escape the handler.
Escaping turns /precheck into an HTTP 500, which the controller reads as PASSED
(controller/preflight.py fail-open) — a plugin-reported failure would silently
become a proceed.
"""
try:
return str(error)
except Exception:
return "<unrenderable error>"
str() on a plugin-raised error can itself raise, replacing the sanitized envelope with a raw HTTP 500 that the controller's preflight treats as fail-open.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Instance __getattribute__ can intercept __class__ access; type() cannot be intercepted, so the class-name interpolation can never escape the handler.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Shadow auto-approve: would require human review. Adds an optional field to public API response models and changes invocation execution (asyncio.to_thread) plus stricter precheck signature validation, altering contract and operational behavior.
Re-trigger cubic
The precheck response is derived from the invoke response internally. Without this a plugin's precheck can't tell the controller why it failed The uvicorn wrapper will be replaced soon so the goal here is the minimal change that unblocks failure reporting on existing plugins without a design of the response models |
2 participants
Summary
failure_categorytoInvokeResponseandInvokePrecheckResponse, populated from afailure_categoryattribute on the raised error (same pattern as the existingstatus_codepickup), and thread it through the/precheckrouteprecheck_func=wiring; today the pydantic response model silently drops the field, forcing plugins that need it to bypass the SDK-installed precheck routeTesting
PYTHONPATH=. uv run pytest test/api/test_api.py— 43 passed, including new coverage for a failing precheck carryingfailure_categoryand a passing precheck leaving it nullruff formatcomplaint inapi_generator.pypredates this change)