fix: return 422 instead of 500 for invalid options on the multipart endpoints - #672
Open
DanielNg0729 wants to merge 1 commit into
Open
Conversation
…ndpoints FormDepends decodes nested models and dict fields from JSON strings itself and reported failures as a plain ValueError, which FastAPI does not translate into a validation response. Rebuilding the model with cls(**newdata) leaked pydantic ValidationError the same way, so field constraints and model validators also crashed the request. Raise RequestValidationError with error locations re-anchored onto the originating form field. Also match PEP 604 unions when detecting nested models and dict fields; get_origin returns types.UnionType rather than typing.Union for X | None, so such fields were never flattened into JSON form fields. Signed-off-by: Daniel Nguyen <danielnguyenh07@gmail.com>
Contributor
|
✅ DCO Check Passed Thanks @DanielNg0729, all your commits are properly signed off. 🎉 |
Contributor
Merge Protections🟢 Merge protection satisfied — ready to merge. Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi maintainers, while running test on #662, i saw a bug so i try to fix it here. Thanks a lot!
Invalid convert/chunk options submitted as
multipart/form-datacrash the request with an unhandled-exception 500 instead of being rejected with a 422. The JSON-body endpoints reject the very same payloads with a 422, so the two request paths disagree on what a client error looks like.FormDependsflattens nested models and dict fields into form parameters that carry JSON strings, and therefore has to validate them itself. It reported every failure as a plainValueError, which FastAPI has no handler for; the model is also rebuilt with a barecls(**newdata), so field constraints andmodel_validatorchecks raise apydantic.ValidationErrorthat escapes the same way. Neither is aRequestValidationError, so FastAPI never recognises them as validation problems.Affected on
/v1/convert/file,/v1/convert/file/async,/v1/chunk/fileand/v1/chunk/file/async:pdf_heading_hierarchy_options,picture_description_local,vlm_pipeline_model_*, …)ocr_custom_config,layout_custom_config, …)picture_description_local+picture_description_api)callbacksvalueErrors are re-anchored onto the form field that carried them, including the
convert_/chunking_prefixes used by the chunk endpoints, so the response points at the field the client actually sent:{ "detail": [ { "type": "int_parsing", "loc": ["body", "convert_pdf_heading_hierarchy_options", "max_level"], "msg": "Input should be a valid integer, unable to parse string as an integer", "input": "banana" } ] }All invalid fields in one request are reported together rather than only the first. Model-level errors omit
input, which on the form path would otherwise echo back the entire options object including every default the caller never sent.Also fixed: PEP 604 unions were not recognised
is_pydantic_model()andis_json_field()matched onlytyping.Union, so a field annotatedSomeOptions | Nonerather thanOptional[SomeOptions]was never flattened into a JSON form field.get_origin()returnstypes.UnionTypefor that spelling, nottyping.Union. No shipped option field uses it today, so this is latent rather than user-visible — but the repo's own lint rules push new code towardX | None, and the next option written that way would silently stop accepting nested values over the form path. Both spellings are now matched.Testing
tests/test_form_options_validation.pycovers each row of the table above, that valid options still round-trip unchanged, that the reported location keeps its form-field prefix, and that both union spellings are detected. The tests fail onmainand pass with this change.