Skip to content

Commit 09bca9c

Browse files
badGarnetclaude
andcommitted
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) <noreply@anthropic.com>
1 parent 78a3211 commit 09bca9c

3 files changed

Lines changed: 47 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@
88
An absent body now resolves each field to its own default, which is what the signature already
99
promised. Plugins with at least one required field are unchanged: a missing `file_data` still
1010
fails validation rather than arriving as `None`.
11+
* **An optional `file_data` no longer 500s when absent.** The wrapper converted `file_data` from its
12+
dict form unconditionally, so a plugin declaring it optional hit
13+
`AttributeError: 'NoneType' object has no attribute 'model_dump'` on any body that omitted it —
14+
previously reachable via `POST {}`, and via a bodyless request once the change above landed. `None`
15+
is now passed through untouched and only a real value is converted.
1116

1217
## 0.0.44
1318

test/api/test_api.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,41 @@ def test_required_param_still_rejects_an_absent_body():
539539
assert client.post("/invoke", json={"element_dicts": "x"}).status_code == 200
540540

541541

542+
class _FileDataEcho(BaseModel):
543+
identifier: Optional[str] = None
544+
545+
546+
def _optional_file_data(
547+
file_data: Optional[FileData] = None, invocation_context: Optional[dict] = None
548+
) -> _FileDataEcho:
549+
return _FileDataEcho(identifier=None if file_data is None else file_data.identifier)
550+
551+
552+
@pytest.mark.parametrize("body", [None, {}])
553+
def test_optional_file_data_is_preserved_as_none(body):
554+
# `file_data` is converted from its dict form for the wrapped function, but it can legitimately
555+
# be absent. Calling `.model_dump()` on None raised before `wrap_fn` ran, turning the
556+
# optional-body contract into a 500 rather than a normal response.
557+
client = TestClient(wrap_in_fastapi(func=_optional_file_data, plugin_id="mock_plugin"))
558+
559+
kwargs = {} if body is None else {"json": body}
560+
resp = client.post("/invoke", **kwargs)
561+
562+
assert resp.status_code == 200
563+
invoke_response = InvokeResponse.model_validate(resp.json())
564+
invoke_response.generic_validation()
565+
assert invoke_response.output == {"identifier": None}
566+
567+
568+
def test_optional_file_data_is_still_converted_when_supplied():
569+
client = TestClient(wrap_in_fastapi(func=_optional_file_data, plugin_id="mock_plugin"))
570+
571+
resp = client.post("/invoke", json={"file_data": mock_file_data[0].model_dump()})
572+
573+
assert resp.status_code == 200
574+
assert InvokeResponse.model_validate(resp.json()).output == {"identifier": "mock file data"}
575+
576+
542577
def test_no_param_plugin_still_accepts_a_bodyless_post():
543578
client = TestClient(wrap_in_fastapi(func=_no_params, plugin_id="mock_plugin"))
544579

unstructured_platform_plugins/etl_uvicorn/api_generator.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,13 @@ async def run_job_with_body(request: BaseModel) -> ResponseType:
258258
log_func_and_body(func=func, body=request.json())
259259
# Create dictionary from pydantic model while preserving underlying types
260260
request_dict = {f: getattr(request, f) for f in request.model_fields}
261-
# Make sure nested classes get instantiated correctly
262-
if "file_data" in request_dict:
263-
request_dict["file_data"] = file_data_from_dict(request_dict["file_data"].model_dump())
261+
# Make sure nested classes get instantiated correctly. `file_data` can legitimately be None
262+
# -- a plugin may declare it optional, and then an absent or partial body leaves it unset --
263+
# so convert only a real value. Calling `.model_dump()` on None would raise before `wrap_fn`
264+
# runs, turning the optional-body contract into a 500.
265+
file_data = request_dict.get("file_data")
266+
if file_data is not None:
267+
request_dict["file_data"] = file_data_from_dict(file_data.model_dump())
264268
map_inputs(func=func, raw_inputs=request_dict)
265269
if logger.level == LOG_LEVELS.get("trace", logging.NOTSET):
266270
logger.log(level=logger.level, msg=f"passing inputs to function: {request_dict}")

0 commit comments

Comments
 (0)