From 78a321188221b01ed540ea2495f36d2c95c792a7 Mon Sep 17 00:00:00 2001 From: Yao You Date: Tue, 28 Jul 2026 15:21:00 -0500 Subject: [PATCH 1/2] fix(etl-uvicorn): do not require a body when every input field is optional A pydantic body parameter with no default is mandatory even when every field inside the model is optional. `wrap_in_fastapi` chose its `/invoke` signature on parameter *presence*, so a plugin whose parameters are all optional got a required body that no caller has a reason to populate -- and before it grew those parameters the same plugin accepted no body at all. Adding an optional parameter therefore looked backward-compatible while flipping the HTTP contract to 422 for every bodyless caller. Observed in production: the playground indexer gained `invocation_settings`/`invocation_context` (both defaulting to None) and every ephemeral job began failing with [{"type":"missing","loc":["body"],"msg":"Field required","input":null}] The indexer is the first node in the DAG and the source of all documents, so nothing was indexed, every downstream node idled, and the job still reported COMPLETED -- with total_docs 0 and an empty failed-files list. An absent body now resolves each field to its own default, which is what the signature already promised. Plugins with at least one required field keep a mandatory body, so a downloader invoked without `file_data` still fails validation rather than receiving None. The two model-bearing branches share one handler so they cannot drift. Verified against the real playground indexer: a bodyless POST returns 200 and indexes from the settings-file fallback, while a populated body still takes precedence, so the wire-settings migration keeps working. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 11 +++ test/api/test_api.py | 76 +++++++++++++++++++ unstructured_platform_plugins/__version__.py | 2 +- .../etl_uvicorn/api_generator.py | 48 ++++++++---- 4 files changed, 123 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79da33b..c5dce23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ +## 0.0.45 + +* **`/invoke` no longer demands a body from a plugin whose parameters are all optional.** A pydantic + body parameter with no default is mandatory even when every field inside the model is optional, so + such a plugin required a body that no caller has a reason to populate — and before it grew those + parameters the same plugin accepted no body at all, which made adding one look + backward-compatible while silently flipping the HTTP contract to 422 for every bodyless caller. + An absent body now resolves each field to its own default, which is what the signature already + promised. Plugins with at least one required field are unchanged: a missing `file_data` still + fails validation rather than arriving as `None`. + ## 0.0.44 * **Ignore SIGTERM in plugin uvicorn Servers**: plugin webservers now keep diff --git a/test/api/test_api.py b/test/api/test_api.py index f8bca3c..993b016 100644 --- a/test/api/test_api.py +++ b/test/api/test_api.py @@ -470,3 +470,79 @@ def test_streaming_unstructured_ingest_error_with_none_status_code(): "Async gen test UnstructuredIngestError with None status_code" in invoke_response.status_code_text ) + + +# --- optional-body contract ------------------------------------------------------------------- +# +# A pydantic body parameter with no default is mandatory even when every field inside the model is +# optional. A plugin whose parameters are ALL optional therefore used to demand a body that no +# caller has a reason to populate: before it grew those parameters the same plugin accepted no body +# at all, so adding one silently flipped its HTTP contract and every bodyless caller got a 422. + + +class _Echo(BaseModel): + settings: Optional[dict] = None + context: Optional[dict] = None + received: Optional[str] = None + + +def _all_optional( + invocation_settings: Optional[dict] = None, invocation_context: Optional[dict] = None +) -> _Echo: + return _Echo(settings=invocation_settings, context=invocation_context) + + +def _no_params() -> _Echo: + return _Echo(received="ok") + + +def _has_required(element_dicts: str, invocation_context: Optional[dict] = None) -> _Echo: + return _Echo(received=element_dicts) + + +@pytest.mark.parametrize("body", [None, {}]) +def test_all_optional_params_accept_absent_or_empty_body(body): + client = TestClient(wrap_in_fastapi(func=_all_optional, plugin_id="mock_plugin")) + + kwargs = {} if body is None else {"json": body} + resp = client.post("/invoke", **kwargs) + + assert resp.status_code == 200 + invoke_response = InvokeResponse.model_validate(resp.json()) + invoke_response.generic_validation() + # Each field resolves to its own default, which is what the signature already promised. + assert invoke_response.output == {"settings": None, "context": None, "received": None} + + +def test_all_optional_params_still_receive_a_populated_body(): + # The tolerance must not swallow a body that IS supplied, or the wire-settings plane silently + # stops working while every request keeps returning 200. + client = TestClient(wrap_in_fastapi(func=_all_optional, plugin_id="mock_plugin")) + + resp = client.post("/invoke", json={"invocation_settings": {"k": "v"}}) + + assert resp.status_code == 200 + assert InvokeResponse.model_validate(resp.json()).output == { + "settings": {"k": "v"}, + "context": None, + "received": None, + } + + +def test_required_param_still_rejects_an_absent_body(): + # The tolerance must not leak into plugins that genuinely need input: a downloader invoked + # without `file_data` has to fail loudly rather than receive None. + client = TestClient(wrap_in_fastapi(func=_has_required, plugin_id="mock_plugin")) + + assert client.post("/invoke").status_code == 422 + assert client.post("/invoke", json={}).status_code == 422 + assert client.post("/invoke", json={"element_dicts": "x"}).status_code == 200 + + +def test_no_param_plugin_still_accepts_a_bodyless_post(): + client = TestClient(wrap_in_fastapi(func=_no_params, plugin_id="mock_plugin")) + + resp = client.post("/invoke") + + assert resp.status_code == 200 + assert InvokeResponse.model_validate(resp.json()).output["received"] == "ok" diff --git a/unstructured_platform_plugins/__version__.py b/unstructured_platform_plugins/__version__.py index 3fbbe28..d8f2458 100644 --- a/unstructured_platform_plugins/__version__.py +++ b/unstructured_platform_plugins/__version__.py @@ -1 +1 @@ -__version__ = "0.0.44" # pragma: no cover +__version__ = "0.0.45" # pragma: no cover diff --git a/unstructured_platform_plugins/etl_uvicorn/api_generator.py b/unstructured_platform_plugins/etl_uvicorn/api_generator.py index 06e0074..cb64ee1 100644 --- a/unstructured_platform_plugins/etl_uvicorn/api_generator.py +++ b/unstructured_platform_plugins/etl_uvicorn/api_generator.py @@ -254,22 +254,44 @@ async def _stream_response(): file_data=request_dict.get("file_data", None), ) - if input_schema_model.model_fields: + async def run_job_with_body(request: BaseModel) -> ResponseType: + log_func_and_body(func=func, body=request.json()) + # Create dictionary from pydantic model while preserving underlying types + request_dict = {f: getattr(request, f) for f in request.model_fields} + # Make sure nested classes get instantiated correctly + if "file_data" in request_dict: + request_dict["file_data"] = file_data_from_dict(request_dict["file_data"].model_dump()) + map_inputs(func=func, raw_inputs=request_dict) + if logger.level == LOG_LEVELS.get("trace", logging.NOTSET): + logger.log(level=logger.level, msg=f"passing inputs to function: {request_dict}") + return await wrap_fn(func=func, kwargs=request_dict) + + # A pydantic body parameter with no default is mandatory even when every field inside the model + # is optional. So a plugin whose parameters are ALL optional would demand a body that no caller + # has a reason to populate -- and before it grew those parameters that same plugin accepted no + # body at all, so adding one flips the HTTP contract while looking backward-compatible. Default + # the body in that case: an absent body resolves each field to its own default, which is exactly + # what the function signature already promises. + # + # A plugin with at least one required field keeps a mandatory body, so an indexer that needs + # `file_data` still fails validation rather than silently receiving None. + body_is_optional = input_schema_model.model_fields and not any( + field.is_required() for field in input_schema_model.model_fields.values() + ) + + if body_is_optional: + + @fastapi_app.post("/invoke", response_model=InvokeResponse) + async def run_job(request: Optional[input_schema_model] = None) -> ResponseType: + return await run_job_with_body( + request if request is not None else input_schema_model() + ) + + elif input_schema_model.model_fields: @fastapi_app.post("/invoke", response_model=InvokeResponse) async def run_job(request: input_schema_model) -> ResponseType: - log_func_and_body(func=func, body=request.json()) - # Create dictionary from pydantic model while preserving underlying types - request_dict = {f: getattr(request, f) for f in request.model_fields} - # Make sure nested classes get instantiated correctly - if "file_data" in request_dict: - request_dict["file_data"] = file_data_from_dict( - request_dict["file_data"].model_dump() - ) - map_inputs(func=func, raw_inputs=request_dict) - if logger.level == LOG_LEVELS.get("trace", logging.NOTSET): - logger.log(level=logger.level, msg=f"passing inputs to function: {request_dict}") - return await wrap_fn(func=func, kwargs=request_dict) + return await run_job_with_body(request) else: From 09bca9c3ad9475f2cba897e75cb062b04764d356 Mon Sep 17 00:00:00 2001 From: Yao You Date: Tue, 28 Jul 2026 15:38:44 -0500 Subject: [PATCH 2/2] fix(etl-uvicorn): preserve a None file_data instead of dumping it Review found that `run_job_with_body` converted `file_data` from its dict form unconditionally, so a plugin declaring `file_data` optional raised AttributeError: 'NoneType' object has no attribute 'model_dump' before `wrap_fn` could run -- surfacing as a 500 rather than the normal response the signature promises. This predates the optional-body change: it was already reachable on main via `POST {}`, since an omitted field is None whether the body is absent or merely partial. Defaulting the body widens the same hole to bodyless requests, so fix it here rather than leaving a 500 behind the contract this branch is establishing. Pass None through untouched and convert only a real value. A plugin with a required `file_data` is unaffected: validation rejects the request before this line, so the conversion still always runs when the field is declared mandatory. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 5 +++ test/api/test_api.py | 35 +++++++++++++++++++ .../etl_uvicorn/api_generator.py | 10 ++++-- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5dce23..e3a14bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ An absent body now resolves each field to its own default, which is what the signature already promised. Plugins with at least one required field are unchanged: a missing `file_data` still fails validation rather than arriving as `None`. +* **An optional `file_data` no longer 500s when absent.** The wrapper converted `file_data` from its + dict form unconditionally, so a plugin declaring it optional hit + `AttributeError: 'NoneType' object has no attribute 'model_dump'` on any body that omitted it — + previously reachable via `POST {}`, and via a bodyless request once the change above landed. `None` + is now passed through untouched and only a real value is converted. ## 0.0.44 diff --git a/test/api/test_api.py b/test/api/test_api.py index 993b016..83e2b23 100644 --- a/test/api/test_api.py +++ b/test/api/test_api.py @@ -539,6 +539,41 @@ def test_required_param_still_rejects_an_absent_body(): assert client.post("/invoke", json={"element_dicts": "x"}).status_code == 200 +class _FileDataEcho(BaseModel): + identifier: Optional[str] = None + + +def _optional_file_data( + file_data: Optional[FileData] = None, invocation_context: Optional[dict] = None +) -> _FileDataEcho: + return _FileDataEcho(identifier=None if file_data is None else file_data.identifier) + + +@pytest.mark.parametrize("body", [None, {}]) +def test_optional_file_data_is_preserved_as_none(body): + # `file_data` is converted from its dict form for the wrapped function, but it can legitimately + # be absent. Calling `.model_dump()` on None raised before `wrap_fn` ran, turning the + # optional-body contract into a 500 rather than a normal response. + client = TestClient(wrap_in_fastapi(func=_optional_file_data, plugin_id="mock_plugin")) + + kwargs = {} if body is None else {"json": body} + resp = client.post("/invoke", **kwargs) + + assert resp.status_code == 200 + invoke_response = InvokeResponse.model_validate(resp.json()) + invoke_response.generic_validation() + assert invoke_response.output == {"identifier": None} + + +def test_optional_file_data_is_still_converted_when_supplied(): + client = TestClient(wrap_in_fastapi(func=_optional_file_data, plugin_id="mock_plugin")) + + resp = client.post("/invoke", json={"file_data": mock_file_data[0].model_dump()}) + + assert resp.status_code == 200 + assert InvokeResponse.model_validate(resp.json()).output == {"identifier": "mock file data"} + + def test_no_param_plugin_still_accepts_a_bodyless_post(): client = TestClient(wrap_in_fastapi(func=_no_params, plugin_id="mock_plugin")) diff --git a/unstructured_platform_plugins/etl_uvicorn/api_generator.py b/unstructured_platform_plugins/etl_uvicorn/api_generator.py index cb64ee1..4b7d8ce 100644 --- a/unstructured_platform_plugins/etl_uvicorn/api_generator.py +++ b/unstructured_platform_plugins/etl_uvicorn/api_generator.py @@ -258,9 +258,13 @@ async def run_job_with_body(request: BaseModel) -> ResponseType: log_func_and_body(func=func, body=request.json()) # Create dictionary from pydantic model while preserving underlying types request_dict = {f: getattr(request, f) for f in request.model_fields} - # Make sure nested classes get instantiated correctly - if "file_data" in request_dict: - request_dict["file_data"] = file_data_from_dict(request_dict["file_data"].model_dump()) + # Make sure nested classes get instantiated correctly. `file_data` can legitimately be None + # -- a plugin may declare it optional, and then an absent or partial body leaves it unset -- + # so convert only a real value. Calling `.model_dump()` on None would raise before `wrap_fn` + # runs, turning the optional-body contract into a 500. + file_data = request_dict.get("file_data") + if file_data is not None: + request_dict["file_data"] = file_data_from_dict(file_data.model_dump()) map_inputs(func=func, raw_inputs=request_dict) if logger.level == LOG_LEVELS.get("trace", logging.NOTSET): logger.log(level=logger.level, msg=f"passing inputs to function: {request_dict}")