[Bugfix] Add error handling for FINISHED_ERROR in OpenAIServing#37148
[Bugfix] Add error handling for FINISHED_ERROR in OpenAIServing#37148DarkLight1337 merged 1 commit intovllm-project:mainfrom
Conversation
|
Hi @chaunceyjiang, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
There was a problem hiding this comment.
Code Review
This pull request aims to fix an issue where FINISHED_ERROR from the engine was causing unhandled exceptions and noisy logs. The approach of re-introducing error handling to convert GenerationError into a proper HTTP 500 error response is correct. The changes in chat_completion and completion endpoints, along with the new tests, are well-implemented. However, I've found a critical issue in the responses endpoint where the generated error response is not being returned, which would defeat the purpose of the fix for that endpoint.
|
Had no idea this was a thing, can you add a code comment explaining why this is needed? I really thought |
+1. |
|
Btw, please take a look at pr #37157.
|
|
Test |
Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
1216be6 to
c7c0ddd
Compare
@andyxning PTAL. I believe this is necessary. When Therefore, I think the behavior should be consistent regardless of whether |
DarkLight1337
left a comment
There was a problem hiding this comment.
This is cleaner, thanks
|
/lgtm |
…-project#37148) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
…-project#37148) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
…-project#37148) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
…-project#37148) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
Purpose
PR #26813 introduced a
FINISHED_ERRORerror for P/D and converted it into a 500 HTTP error. However, PR #31164 removed this handling, which caused a large number of logs like the following to appear. This may lead users to mistakenly believe that vLLM has encountered an error.Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.