fix(ai): risposta AI malformata -> 502, non 400 (#36 punto 7)
json.JSONDecodeError e' un ValueError: KeyError/JSONDecodeError sulla risposta del modello finivano nell'except ValueError del router (pensato per i nostri errori di configurazione, es. API key mancante) e uscivano come 400 invece di 502. Nuova AIResponseError dedicata in ai_service.py per contenuto malformato o JSON non valido; recipes.py la traduce in 502, toccato solo per questo except. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -189,6 +189,11 @@ async def parse_technical_sheet(
|
|||||||
tasks = await ai_service.parse_technical_sheet(pdf_bytes)
|
tasks = await ai_service.parse_technical_sheet(pdf_bytes)
|
||||||
except ValueError as e:
|
except ValueError as e:
|
||||||
raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))
|
raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))
|
||||||
|
except ai_service.AIResponseError as e:
|
||||||
|
# The AI model's own response was malformed (bad JSON, missing
|
||||||
|
# fields): that is the upstream service's fault, not ours (400).
|
||||||
|
logging.getLogger(__name__).error("AI response malformed: %s", e)
|
||||||
|
raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="Risposta del servizio AI non valida")
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logging.getLogger(__name__).error("AI parsing error: %s", e)
|
logging.getLogger(__name__).error("AI parsing error: %s", e)
|
||||||
raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="Errore nel servizio AI")
|
raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="Errore nel servizio AI")
|
||||||
|
|||||||
@@ -11,6 +11,17 @@ from src.backend.config import settings
|
|||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
class AIResponseError(Exception):
|
||||||
|
"""The AI model's response could not be used: missing fields or invalid JSON.
|
||||||
|
|
||||||
|
Distinct from ValueError (raised for our own misconfiguration, e.g. a
|
||||||
|
missing API key) so the router can tell "we are broken" (400) apart from
|
||||||
|
"the upstream AI service is broken" (502) instead of both landing on the
|
||||||
|
same except clause.
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
SYSTEM_PROMPT = """Sei un assistente specializzato nell'analisi di schede tecniche industriali.
|
SYSTEM_PROMPT = """Sei un assistente specializzato nell'analisi di schede tecniche industriali.
|
||||||
Analizza il testo estratto da una scheda tecnica e identifica i blocchi operativi distinti.
|
Analizza il testo estratto da una scheda tecnica e identifica i blocchi operativi distinti.
|
||||||
Per ogni blocco, estrai:
|
Per ogni blocco, estrai:
|
||||||
@@ -65,7 +76,10 @@ async def parse_technical_sheet(pdf_bytes: bytes) -> list[dict]:
|
|||||||
resp.raise_for_status()
|
resp.raise_for_status()
|
||||||
|
|
||||||
data = resp.json()
|
data = resp.json()
|
||||||
content = data["choices"][0]["message"]["content"].strip()
|
try:
|
||||||
|
content = data["choices"][0]["message"]["content"].strip()
|
||||||
|
except (KeyError, IndexError, TypeError, AttributeError) as e:
|
||||||
|
raise AIResponseError(f"Risposta del modello AI malformata: {e}") from e
|
||||||
|
|
||||||
# Strip markdown fences if present
|
# Strip markdown fences if present
|
||||||
if content.startswith("```"):
|
if content.startswith("```"):
|
||||||
@@ -74,4 +88,10 @@ async def parse_technical_sheet(pdf_bytes: bytes) -> list[dict]:
|
|||||||
content = content[:-3]
|
content = content[:-3]
|
||||||
content = content.strip()
|
content = content.strip()
|
||||||
|
|
||||||
return json.loads(content)
|
try:
|
||||||
|
return json.loads(content)
|
||||||
|
except json.JSONDecodeError as e:
|
||||||
|
# json.JSONDecodeError is itself a ValueError: without this, the
|
||||||
|
# router's `except ValueError` (meant for our own config errors)
|
||||||
|
# would misreport a broken upstream response as our fault (400).
|
||||||
|
raise AIResponseError(f"Risposta del modello AI non e' JSON valido: {e}") from e
|
||||||
|
|||||||
@@ -0,0 +1,67 @@
|
|||||||
|
"""Tests for ai_service.py error handling (gitea#36 point 7).
|
||||||
|
|
||||||
|
A malformed response from the AI model (missing fields, invalid JSON) must
|
||||||
|
raise the dedicated AIResponseError, not a bare KeyError/JSONDecodeError —
|
||||||
|
JSONDecodeError in particular is itself a ValueError subclass, so without
|
||||||
|
this it was caught by the router's `except ValueError` (meant for our own
|
||||||
|
config errors) and misreported as a 400 instead of a 502.
|
||||||
|
"""
|
||||||
|
import json
|
||||||
|
from unittest.mock import AsyncMock, patch
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from src.backend.services import ai_service
|
||||||
|
|
||||||
|
|
||||||
|
class _FakeResponse:
|
||||||
|
def __init__(self, json_data):
|
||||||
|
self._json_data = json_data
|
||||||
|
|
||||||
|
def raise_for_status(self):
|
||||||
|
pass
|
||||||
|
|
||||||
|
def json(self):
|
||||||
|
return self._json_data
|
||||||
|
|
||||||
|
|
||||||
|
def _patch_openrouter(monkeypatch, response_json: dict):
|
||||||
|
monkeypatch.setattr(ai_service, "extract_text_from_pdf", lambda _b: "some extracted text")
|
||||||
|
monkeypatch.setattr(ai_service.settings, "openrouter_api_key", "test-key")
|
||||||
|
monkeypatch.setattr(ai_service.settings, "openrouter_model", "test-model")
|
||||||
|
|
||||||
|
fake_client = AsyncMock()
|
||||||
|
fake_client.post = AsyncMock(return_value=_FakeResponse(response_json))
|
||||||
|
fake_client.__aenter__ = AsyncMock(return_value=fake_client)
|
||||||
|
fake_client.__aexit__ = AsyncMock(return_value=False)
|
||||||
|
|
||||||
|
return patch("src.backend.services.ai_service.httpx.AsyncClient", return_value=fake_client)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_missing_choices_key_raises_ai_response_error(monkeypatch):
|
||||||
|
with _patch_openrouter(monkeypatch, {"no_choices_here": True}):
|
||||||
|
with pytest.raises(ai_service.AIResponseError):
|
||||||
|
await ai_service.parse_technical_sheet(b"%PDF-fake")
|
||||||
|
|
||||||
|
|
||||||
|
async def test_invalid_json_content_raises_ai_response_error(monkeypatch):
|
||||||
|
bad_content = "not valid json {{{"
|
||||||
|
response_json = {"choices": [{"message": {"content": bad_content}}]}
|
||||||
|
with _patch_openrouter(monkeypatch, response_json):
|
||||||
|
with pytest.raises(ai_service.AIResponseError):
|
||||||
|
await ai_service.parse_technical_sheet(b"%PDF-fake")
|
||||||
|
|
||||||
|
|
||||||
|
async def test_valid_json_content_returns_tasks(monkeypatch):
|
||||||
|
tasks = [{"title": "MATERIALI", "directive": "Verificare", "description": "..."}]
|
||||||
|
response_json = {"choices": [{"message": {"content": json.dumps(tasks)}}]}
|
||||||
|
with _patch_openrouter(monkeypatch, response_json):
|
||||||
|
result = await ai_service.parse_technical_sheet(b"%PDF-fake")
|
||||||
|
assert result == tasks
|
||||||
|
|
||||||
|
|
||||||
|
async def test_empty_extracted_text_returns_empty_list(monkeypatch):
|
||||||
|
"""No AI call at all when the PDF has no extractable text."""
|
||||||
|
monkeypatch.setattr(ai_service, "extract_text_from_pdf", lambda _b: " ")
|
||||||
|
result = await ai_service.parse_technical_sheet(b"%PDF-fake")
|
||||||
|
assert result == []
|
||||||
@@ -0,0 +1,33 @@
|
|||||||
|
"""Router-level test for gitea#36 point 7: recipes.py translates
|
||||||
|
ai_service.AIResponseError to 502, not the config-error 400.
|
||||||
|
|
||||||
|
A dedicated file (not test_recipes.py) to avoid colliding with concurrent
|
||||||
|
work on the copy-on-write recipe logic in that shared test module.
|
||||||
|
"""
|
||||||
|
from httpx import AsyncClient
|
||||||
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
|
from src.backend.models.orm.user import User
|
||||||
|
from src.backend.services import ai_service
|
||||||
|
from src.backend.tests.conftest import auth_headers, create_test_recipe
|
||||||
|
|
||||||
|
|
||||||
|
async def test_ai_response_error_is_502(
|
||||||
|
client: AsyncClient,
|
||||||
|
maker_user: User,
|
||||||
|
db_session: AsyncSession,
|
||||||
|
monkeypatch,
|
||||||
|
):
|
||||||
|
recipe = await create_test_recipe(db_session, maker_user.id, code="AI-PARSE-502")
|
||||||
|
|
||||||
|
async def _raise_ai_response_error(_pdf_bytes):
|
||||||
|
raise ai_service.AIResponseError("Risposta del modello AI non e' JSON valido: boom")
|
||||||
|
|
||||||
|
monkeypatch.setattr(ai_service, "parse_technical_sheet", _raise_ai_response_error)
|
||||||
|
|
||||||
|
resp = await client.post(
|
||||||
|
f"/api/recipes/{recipe.id}/parse-technical-sheet",
|
||||||
|
headers=auth_headers(maker_user),
|
||||||
|
files={"file": ("sheet.pdf", b"%PDF-1.4 fake", "application/pdf")},
|
||||||
|
)
|
||||||
|
assert resp.status_code == 502
|
||||||
Reference in New Issue
Block a user