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.
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)