[Bugfix] Replace assert with ValueError for response_format validatio…#35514
[Bugfix] Replace assert with ValueError for response_format validatio…#35514antonovsergey93 wants to merge 1 commit intovllm-project:mainfrom
Conversation
…n in chat completions endpoint Signed-off-by: Sergey Antonov <antonovsergey93@gmail.com>
There was a problem hiding this comment.
Code Review
The code changes include adding a test case to check for a validation error when the json_schema field is missing in the response_format and adding a validator to the ChatCompletionRequest class to ensure that when the response_format type is 'json_schema', the 'json_schema' field is provided. The reviewer commented that the assert statement should be replaced with a more appropriate error handling mechanism, such as raising a ValueError or a custom exception, to provide more informative error messages and prevent the server from crashing.
| structured_outputs_kwargs["json"] = json_schema.json_schema | ||
| elif response_format.type == "structural_tag": | ||
| structural_tag = response_format | ||
| assert structural_tag is not None and isinstance( |
There was a problem hiding this comment.
The assert statement should be replaced with a more appropriate error handling mechanism, such as raising a ValueError or a custom exception, to provide more informative error messages and prevent the server from crashing. This is especially important in production environments.
raise ValueError("structural_tag cannot be None when response_format type is 'structural_tag'")|
Sorry, #35510 came first |
Purpose
When the /v1/chat/completions endpoint receives a request with
response_formattypejson_schemabut without the required json_schema field, the server crashes with an AssertionError, resulting in a 500 Internal Server Error.Fixes #35438
This is the same class of issue addressed in #35456 for the
/v1/completionsendpointTest Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.