Skip to content

Commit c4fe4a2

Browse files
authored
fix: handle non-JSON error responses from API gateway (#741)
fix: handle transient API errors gracefully Wrap three types of transient API errors as PyViCareInternalServerError so the cached service can fall back to stale data instead of crashing with full tracebacks in the HA logs: 1. Non-JSON 5xx responses (e.g. 502 HTML from load balancer) 2. TIMEOUT errors in extendedPayload (empty errorType, no statusCode) 3. Connection-level errors (DNS timeout, connection refused)
1 parent aae372b commit c4fe4a2

2 files changed

Lines changed: 86 additions & 4 deletions

File tree

PyViCare/PyViCareAbstractOAuthManager.py

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ def renewToken(self) -> None:
4444
def get(self, url: str) -> Any:
4545
try:
4646
logger.debug(self.__oauth)
47-
response = self.__oauth.get(f"{API_BASE_URL}{url}", timeout=31).json()
47+
raw_response = self.__oauth.get(f"{API_BASE_URL}{url}", timeout=31)
48+
self.__raise_on_non_json_error(raw_response)
49+
response = raw_response.json()
4850
logger.debug("Response to get request: %s", response)
4951
self.__handle_expired_token(response)
5052
self.__handle_rate_limit(response)
@@ -58,6 +60,21 @@ def get(self, url: str) -> Any:
5860
except InvalidTokenError:
5961
self.renewToken()
6062
return self.get(url)
63+
except OSError as e:
64+
raise PyViCareInternalServerError(
65+
{"statusCode": 0,
66+
"message": str(e),
67+
"viErrorId": "n/a"}) from e
68+
69+
def __raise_on_non_json_error(self, response):
70+
"""Guard against non-JSON error responses (e.g. 502 HTML pages from API gateway)."""
71+
if response.status_code >= 500:
72+
content_type = response.headers.get('content-type', '')
73+
if 'application/json' not in content_type:
74+
raise PyViCareInternalServerError(
75+
{"statusCode": response.status_code,
76+
"message": f"Non-JSON {response.status_code} response",
77+
"viErrorId": "n/a"})
6178

6279
def __handle_expired_token(self, response):
6380
if ("error" in response and response["error"] == "EXPIRED TOKEN"):
@@ -82,6 +99,13 @@ def __handle_server_error(self, response):
8299
if ("statusCode" in response and response["statusCode"] >= 500):
83100
raise PyViCareInternalServerError(response)
84101

102+
extended = response.get("extendedPayload", {})
103+
if isinstance(extended, dict) and extended.get("code") in ("500", "502", "503"):
104+
raise PyViCareInternalServerError(
105+
{"statusCode": int(extended["code"]),
106+
"message": extended.get("reason", ""),
107+
"viErrorId": response.get("viErrorId", "n/a")})
108+
85109
def __handle_command_error(self, response):
86110
if not Feature.raise_exception_on_command_failure:
87111
return
@@ -106,8 +130,10 @@ def post(self, url, data) -> Any:
106130
headers = {"Content-Type": "application/json",
107131
"Accept": "application/vnd.siren+json"}
108132
try:
109-
response = self.__oauth.post(
110-
f"{API_BASE_URL}{url}", data, headers=headers).json()
133+
raw_response = self.__oauth.post(
134+
f"{API_BASE_URL}{url}", data, headers=headers)
135+
self.__raise_on_non_json_error(raw_response)
136+
response = raw_response.json()
111137
self.__handle_expired_token(response)
112138
self.__handle_rate_limit(response)
113139
self.__handle_command_error(response)
@@ -118,3 +144,8 @@ def post(self, url, data) -> Any:
118144
except InvalidTokenError:
119145
self.renewToken()
120146
return self.post(url, data)
147+
except OSError as e:
148+
raise PyViCareInternalServerError(
149+
{"statusCode": 0,
150+
"message": str(e),
151+
"viErrorId": "n/a"}) from e

tests/test_ViCareOAuthManager.py

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@ def renewToken(self):
1818

1919

2020
class FakeResponse:
21-
def __init__(self, file_name):
21+
def __init__(self, file_name, status_code=200, content_type='application/json'):
2222
self.file_name = file_name
23+
self.status_code = status_code
24+
self.headers = {'content-type': content_type}
2325

2426
def json(self):
2527
return readJson(self.file_name)
@@ -87,6 +89,55 @@ def func():
8789
return self.manager.post("/", "some")
8890
self.assertRaises(PyViCareRateLimitError, func)
8991

92+
def test_get_raise_on_non_json_502(self):
93+
response = Mock()
94+
response.status_code = 502
95+
response.headers = {'content-type': 'text/html'}
96+
self.oauth_mock.get.return_value = response
97+
98+
def func():
99+
return self.manager.get("/")
100+
self.assertRaises(PyViCareInternalServerError, func)
101+
102+
def test_get_raise_on_extended_payload_timeout(self):
103+
self.oauth_mock.get.return_value = FakeResponse.__new__(FakeResponse)
104+
self.oauth_mock.get.return_value.status_code = 200
105+
self.oauth_mock.get.return_value.headers = {'content-type': 'application/json'}
106+
self.oauth_mock.get.return_value.json = lambda: {
107+
'viErrorId': '00-abc-def-00',
108+
'errorType': '',
109+
'message': '',
110+
'extendedPayload': {'code': '500', 'reason': 'TIMEOUT'}
111+
}
112+
113+
def func():
114+
return self.manager.get("/")
115+
self.assertRaises(PyViCareInternalServerError, func)
116+
117+
def test_get_raise_on_connection_error(self):
118+
self.oauth_mock.get.side_effect = OSError("Timeout while contacting DNS servers")
119+
120+
def func():
121+
return self.manager.get("/")
122+
self.assertRaises(PyViCareInternalServerError, func)
123+
124+
def test_post_raise_on_connection_error(self):
125+
self.oauth_mock.post.side_effect = OSError("Connection refused")
126+
127+
def func():
128+
return self.manager.post("/", {})
129+
self.assertRaises(PyViCareInternalServerError, func)
130+
131+
def test_post_raise_on_non_json_502(self):
132+
response = Mock()
133+
response.status_code = 502
134+
response.headers = {'content-type': 'text/html'}
135+
self.oauth_mock.post.return_value = response
136+
137+
def func():
138+
return self.manager.post("/", {})
139+
self.assertRaises(PyViCareInternalServerError, func)
140+
90141
def test_post_renewtoken_ifexpired(self):
91142
self.oauth_mock.post.side_effect = [
92143
FakeResponse('response/errors/expired_token.json'), # first call expired

0 commit comments

Comments
 (0)