[Bugfix] Return 400 for Qwen image edit validation errors#2930
[Bugfix] Return 400 for Qwen image edit validation errors#2930david6666666 wants to merge 2 commits into
Conversation
Signed-off-by: david6666666 <530634352@qq.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
SamitHuang
left a comment
There was a problem hiding this comment.
The PR addresses the immediate regression by hardening config checks and adding tests. However, there are two issues to resolve. First, vllm_omni/entrypoints/openai/serving_chat.py contains a similar getattr(od_config, "supports_multimodal_inputs", False) pattern that may also fail with non-concrete configs. Second, relying on regex and string matching to parse worker exceptions is brittle and fixes a symptom rather than the root cause.
| logger.error(f"Validation error: {e}") | ||
| raise HTTPException(status_code=HTTPStatus.BAD_REQUEST.value, detail=str(e)) | ||
| except Exception as e: | ||
| image_input_validation_detail = _extract_image_input_validation_detail(e) |
There was a problem hiding this comment.
Relying on regex and string matching of the RuntimeError message is brittle and fixes the symptom rather than the root cause. Please consider raising a specific validation exception type (e.g., ValueError or a custom InputValidationError) from the worker or engine layer so the API layer can catch it cleanly without parsing strings.
| return None | ||
|
|
||
| if not bool(getattr(od_config, "supports_multimodal_inputs", False)): | ||
| supports_multimodal_inputs = _normalize_optional_bool(getattr(od_config, "supports_multimodal_inputs", None)) |
There was a problem hiding this comment.
I noticed a similar pattern exists in vllm_omni/entrypoints/openai/serving_chat.py (around line 2174) where getattr(od_config, "supports_multimodal_inputs", False) is used. If that code path can also be hit with mocked or non-concrete configs during testing or ComfyUI integration, it will suffer from the same bug. Please apply _normalize_optional_bool there as well if applicable.
Signed-off-by: david6666666 <530634352@qq.com>
|
wait for #2426 |
hsliuustc0106
left a comment
There was a problem hiding this comment.
BLOCKER scan: PASS
Good fix with proper error handling and comprehensive tests. The serialized error type approach preserves error information across the pipeline while avoiding brittle validation assumptions.
Summary
/v1/images/editsmulti-image validation so the server only trusts concrete diffusion capability metadata400 Bad Requestinstead of surfacing them as500Root Cause
Issue #2886 is a real code regression introduced around #2840 rather than a bad test.
PR #2840 added early multi-image validation for Qwen image edit requests, but the serving layer assumed that
od_config.supports_multimodal_inputsandod_config.max_multimodal_image_inputswere always concretebool/intvalues. In compatibility and mocked surfaces such as the ComfyUI integration tests, those attributes can be absent or represented by non-concrete placeholder objects. That made the pre-check brittle and allowed invalid exceptions to escape as internal server errors.There was also still a second failure mode where a deeper worker-side validation error like
Received 5 input images...could bubble up as a generic500, even though it is fundamentally a request validation problem and should be reported as400.Why This Fix
400) rather than server errors (500)Validation
pytest -q tests/entrypoints/openai_api/test_image_server.pypytest -q tests/comfyui/test_comfyui_integration.pypre-commit run --all-filesQwen-Image-Edit-2511Test Result
tests/entrypoints/openai_api/test_image_server.py:53 passedtests/comfyui/test_comfyui_integration.py:18 passedpre-commit run --all-files: passedQwen-Image-Edit-2511:/v1/images/editsrequest returned400withReceived 5 input images. At most 4 images are supported by this model./v1/images/editsrequest returned200with a validb64_jsonimage payloadFixes #2886.