Handle yente transport failures gracefully - #6
Conversation
ruthdelalucha
left a comment
There was a problem hiding this comment.
Review: Yente Error Handling ✅
APPROVED
Exzellente Fehlerbehandlung für yente-Transport-Fehler.
Was gut ist:
| Exception | HTTP Status | Bedeutung |
|---|---|---|
| 503 | yente nicht erreichbar | |
| 504 | yente timeout | |
| 503 | andere Request-Fehler | |
| 502 | yente liefert HTTP Fehler |
Best Practices erfüllt:
- ✅ Korrekte HTTP Status Codes (502/503/504 für Gateway-Probleme)
- ✅ Aussagekräftige Fehlermeldungen mit YENTE_URL
- ✅ für saubere Exception-Chains
- ✅ Regression Test für Timeout-Pfad
- ✅ Ruff-Compliant (laut PR Beschreibung)
Testing
Der Test ist präzise und prüft sowohl Status als auch Fehlermeldung.
LGTM – Ready to merge! 🚀
ruthdelalucha
left a comment
There was a problem hiding this comment.
APPROVED - Saubere Fehlerbehandlung fuer yente-Transport-Fehler mit korrekten HTTP Status Codes (502/503/504). Regression Test enthalten. LGTM!
jsg-claude
left a comment
There was a problem hiding this comment.
PR #6 Review — Handle yente transport failures gracefully
Kurz: ✅ Sauber, kann gemergt werden.
Was der PR macht
Bisher wurde bei Verbindungsproblemen zu yente fast alles als 503 oder 500 zurückgegeben. Der PR differenziert korrekt:
Das ist semantisch korrekt — 504 für Timeouts ist HTTP-Standard und hilft Clients zwischen "Service down" und "Service überlastet" zu unterscheiden.
from e — kleine aber wichtige Änderung
Alle raise HTTPException(...) from e behalten jetzt die Original-Exception als Ursache. Das verbessert Tracebacks im Log erheblich.
Test
Der neue test_screen_timeout_returns_gateway_timeout testet genau den richtigen Pfad — httpx.ReadTimeout ist eine Unterklasse von TimeoutException, der Test prüft Status-Code und Response-Body. Sauber.
Einzige Anmerkung
httpx.TimeoutException deckt ConnectTimeout, ReadTimeout, WriteTimeout und PoolTimeout ab. ConnectTimeout ist aber auch eine Unterklasse von ConnectError. Die Reihenfolge der except-Blöcke ist korrekt — TimeoutException steht vor RequestError (dessen Unterklasse es ist), also greift der spezifischere Handler zuerst. ✅
Summary
Testing