[Bugfix] Return 4xx for client-caused errors in /detokenize (#52622)
Signed-off-by: rajathpi <rajathpai2000@gmail.com>
This commit is contained in:
@@ -1,20 +1,27 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
|
||||
|
||||
from argparse import Namespace
|
||||
from dataclasses import dataclass, field
|
||||
from http import HTTPStatus
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from vllm.config.multimodal import MultiModalConfig
|
||||
from vllm.entrypoints.openai.models.protocol import BaseModelPath
|
||||
from vllm.entrypoints.openai.models.serving import OpenAIServingModels
|
||||
from vllm.entrypoints.serve.exception_handling.register import init_exception_handler
|
||||
from vllm.entrypoints.serve.tokenize.api_router import attach_router
|
||||
from vllm.entrypoints.serve.tokenize.protocol import (
|
||||
TokenizeChatRequest,
|
||||
TokenizeCompletionRequest,
|
||||
)
|
||||
from vllm.entrypoints.serve.tokenize.serving import ServingTokenization
|
||||
from vllm.exceptions import VLLMNotFoundError, VLLMValidationError
|
||||
from vllm.renderers.online_renderer import OnlineRenderer
|
||||
from vllm.v1.engine.async_llm import AsyncLLM
|
||||
|
||||
@@ -133,3 +140,109 @@ async def test_tokenize_completion_skips_mm_cache_for_renderer_only_path():
|
||||
serving.online_renderer.preprocess_completion.call_args.kwargs["skip_mm_cache"]
|
||||
is True
|
||||
)
|
||||
|
||||
|
||||
class TestDetokenizeClientErrorResponses:
|
||||
"""Client-caused errors from /detokenize are 4xx, not 500.
|
||||
|
||||
/detokenize used to wrap ``create_detokenize`` in a blanket
|
||||
``except Exception`` that reported every failure as 500, while its
|
||||
sibling /tokenize let exceptions propagate to the global exception
|
||||
handlers, which map client-caused errors to 4xx and keep genuine
|
||||
server errors at 500. These tests pin the shared contract for both
|
||||
endpoints. See #52246 for the same fix in the Anthropic entrypoint.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _make_api_app(handler: MagicMock) -> FastAPI:
|
||||
app = FastAPI()
|
||||
app.state.args = Namespace(log_error_stack=False)
|
||||
app.state.serving_tokenization = handler
|
||||
attach_router(app)
|
||||
init_exception_handler(app)
|
||||
return app
|
||||
|
||||
def _post(self, handler: MagicMock, path: str = "/detokenize"):
|
||||
app = self._make_api_app(handler)
|
||||
body: dict[str, Any] = {"model": "test-model"}
|
||||
if path == "/detokenize":
|
||||
body["tokens"] = [1, 2, 3]
|
||||
else:
|
||||
body["prompt"] = "Hello"
|
||||
with TestClient(app, raise_server_exceptions=False) as client:
|
||||
return client.post(path, json=body)
|
||||
|
||||
def test_vllm_validation_error_returns_bad_request(self):
|
||||
handler = MagicMock(spec=ServingTokenization)
|
||||
handler.create_detokenize.side_effect = VLLMValidationError(
|
||||
"invalid token ids", parameter="tokens"
|
||||
)
|
||||
|
||||
response = self._post(handler)
|
||||
|
||||
assert response.status_code == HTTPStatus.BAD_REQUEST
|
||||
error = response.json()["error"]
|
||||
assert error["type"] == "BadRequestError"
|
||||
assert error["param"] == "tokens"
|
||||
|
||||
def test_vllm_not_found_error_returns_not_found(self):
|
||||
handler = MagicMock(spec=ServingTokenization)
|
||||
handler.create_detokenize.side_effect = VLLMNotFoundError(
|
||||
"LoRA adapter nonexistent-lora not found"
|
||||
)
|
||||
|
||||
response = self._post(handler)
|
||||
|
||||
assert response.status_code == HTTPStatus.NOT_FOUND
|
||||
assert response.json()["error"]["type"] == "NotFoundError"
|
||||
|
||||
def test_value_error_returns_bad_request(self):
|
||||
handler = MagicMock(spec=ServingTokenization)
|
||||
handler.create_detokenize.side_effect = ValueError("bad input")
|
||||
|
||||
response = self._post(handler)
|
||||
|
||||
assert response.status_code == HTTPStatus.BAD_REQUEST
|
||||
assert response.json()["error"]["type"] == "BadRequestError"
|
||||
|
||||
def test_overflow_error_returns_bad_request(self):
|
||||
"""Fast tokenizers raise OverflowError for out-of-range token ids.
|
||||
|
||||
This used to be special-cased into a RequestValidationError; the
|
||||
global OverflowError handler keeps the same 400 status code.
|
||||
"""
|
||||
handler = MagicMock(spec=ServingTokenization)
|
||||
handler.create_detokenize.side_effect = OverflowError(
|
||||
"out of range integral type conversion attempted"
|
||||
)
|
||||
|
||||
response = self._post(handler)
|
||||
|
||||
assert response.status_code == HTTPStatus.BAD_REQUEST
|
||||
assert response.json()["error"]["type"] == "BadRequestError"
|
||||
|
||||
def test_server_error_still_returns_internal_server_error(self):
|
||||
"""Genuine server bugs keep the existing 500 behaviour."""
|
||||
handler = MagicMock(spec=ServingTokenization)
|
||||
handler.create_detokenize.side_effect = RuntimeError("boom")
|
||||
|
||||
response = self._post(handler)
|
||||
|
||||
assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR
|
||||
assert response.json()["error"]["type"] == "InternalServerError"
|
||||
|
||||
def test_tokenize_and_detokenize_agree_on_client_errors(self):
|
||||
"""The two sibling endpoints report the same client error alike."""
|
||||
statuses = {}
|
||||
for path, method in (
|
||||
("/tokenize", "create_tokenize"),
|
||||
("/detokenize", "create_detokenize"),
|
||||
):
|
||||
handler = MagicMock(spec=ServingTokenization)
|
||||
getattr(handler, method).side_effect = VLLMValidationError(
|
||||
"invalid input", parameter="model"
|
||||
)
|
||||
statuses[path] = self._post(handler, path=path).status_code
|
||||
|
||||
assert statuses["/tokenize"] == HTTPStatus.BAD_REQUEST
|
||||
assert statuses["/detokenize"] == HTTPStatus.BAD_REQUEST
|
||||
|
||||
@@ -4,8 +4,7 @@
|
||||
|
||||
from http import HTTPStatus
|
||||
|
||||
from fastapi import APIRouter, Depends, FastAPI, HTTPException, Request
|
||||
from fastapi.exceptions import RequestValidationError
|
||||
from fastapi import APIRouter, Depends, FastAPI, Request
|
||||
from fastapi.responses import JSONResponse
|
||||
from typing_extensions import assert_never
|
||||
|
||||
@@ -72,14 +71,7 @@ async def tokenize(request: TokenizeRequest, raw_request: Request):
|
||||
async def detokenize(request: DetokenizeRequest, raw_request: Request):
|
||||
handler = tokenization(raw_request)
|
||||
|
||||
try:
|
||||
generator = await handler.create_detokenize(request, raw_request)
|
||||
except OverflowError as e:
|
||||
raise RequestValidationError(errors=[str(e)]) from e
|
||||
except Exception as e:
|
||||
raise HTTPException(
|
||||
status_code=HTTPStatus.INTERNAL_SERVER_ERROR.value, detail=str(e)
|
||||
) from e
|
||||
generator = await handler.create_detokenize(request, raw_request)
|
||||
|
||||
if isinstance(generator, ErrorResponse):
|
||||
return JSONResponse(
|
||||
|
||||
Reference in New Issue
Block a user