fix: handle DEVICE_COMMUNICATION_ERROR and return stale cache on transient errors (#704)

* Handle DEVICE_COMMUNICATION_ERROR and return stale cache on transient errors

Add PyViCareDeviceCommunicationError for API responses with
errorType "DEVICE_COMMUNICATION_ERROR" (device/gateway offline).
Previously these were misclassified as PyViCareInvalidDataError.

In the cached service, catch transient errors (device communication
errors and internal server errors) and return stale cache data when
available, only raising if no cache exists. This prevents callers
from crashing on temporary API failures while rate limiting behavior
is preserved.

Fixes #361

* Match existing key check pattern for errorType

Author: lackas

PyViCare/PyViCareAbstractOAuthManager.py

@@ -7,6 +7,7 @@
 
 from PyViCare import Feature
 from PyViCare.PyViCareUtils import (PyViCareCommandError,
+                                    PyViCareDeviceCommunicationError,
                                     PyViCareInternalServerError,
                                     PyViCareRateLimitError)
 
@@ -39,6 +40,7 @@ def get(self, url: str) -> Any:
             logger.debug("Response to get request: %s", response)
             self.__handle_expired_token(response)
             self.__handle_rate_limit(response)
+            self.__handle_device_communication_error(response)
             self.__handle_server_error(response)
             return response
         except TokenExpiredError:
@@ -59,6 +61,10 @@ def __handle_rate_limit(self, response):
         if ("statusCode" in response and response["statusCode"] == 429):
             raise PyViCareRateLimitError(response)
 
+    def __handle_device_communication_error(self, response):
+        if ("errorType" in response and response["errorType"] == "DEVICE_COMMUNICATION_ERROR"):
+            raise PyViCareDeviceCommunicationError(response)
+
     def __handle_server_error(self, response):
         if ("statusCode" in response and response["statusCode"] >= 500):
             raise PyViCareInternalServerError(response)

PyViCare/PyViCareCachedService.py

@@ -5,7 +5,10 @@
 from PyViCare.PyViCareAbstractOAuthManager import AbstractViCareOAuthManager
 from PyViCare.PyViCareService import (ViCareDeviceAccessor, ViCareService,
                                       readFeature)
-from PyViCare.PyViCareUtils import PyViCareInvalidDataError, ViCareTimer
+from PyViCare.PyViCareUtils import (PyViCareDeviceCommunicationError,
+                                    PyViCareInvalidDataError,
+                                    PyViCareInternalServerError,
+                                    ViCareTimer)
 
 logger = logging.getLogger('ViCare')
 logger.addHandler(logging.NullHandler())
@@ -33,13 +36,20 @@ def setProperty(self, property_name, action, data):
     def __get_or_update_cache(self):
         with self.__lock:
             if self.is_cache_invalid():
-                # we always sett the cache time before we fetch the data
+                # we always set the cache time before we fetch the data
                 # to avoid consuming all the api calls if the api is down
                 # see https://github.com/home-assistant/core/issues/67052
                 # we simply return the old cache in this case
                 self.__cacheTime = ViCareTimer().now()
 
-                data = self.fetch_all_features()
+                try:
+                    data = self.fetch_all_features()
+                except (PyViCareDeviceCommunicationError, PyViCareInternalServerError) as e:
+                    if self.__cache is not None:
+                        logger.warning("API error, returning stale cache: %s", e)
+                        return self.__cache
+                    raise
+
                 if "data" not in data:
                     logger.error("Missing 'data' property when fetching data.")
                     raise PyViCareInvalidDataError(data)

PyViCare/PyViCareUtils.py

@@ -105,6 +105,18 @@ class PyViCareInvalidDataError(Exception):
     pass
 
 
+class PyViCareDeviceCommunicationError(Exception):
+    def __init__(self, response):
+        extended_payload = response.get("extendedPayload", {})
+        reason = extended_payload.get("reason", "Unknown")
+
+        msg = f'Device communication error. Reason: {reason}'
+
+        super().__init__(self, msg)
+        self.message = msg
+        self.reason = reason
+
+
 class PyViCareRateLimitError(Exception):
 
     def __init__(self, response):

tests/response/errors/device_offline.json

@@ -0,0 +1,11 @@
+{
+  "viErrorId": "00-1083dbcd84b0d4d6df2ae553dda5c7d2-5ad4674667af26df-00",
+  "statusCode": 400,
+  "errorType": "DEVICE_COMMUNICATION_ERROR",
+  "message": "Device communication error",
+  "extendedPayload": {
+    "httpStatusCode": "NotFound",
+    "code": "404",
+    "reason": "DEVICE_OFFLINE"
+  }
+}

tests/test_PyViCareCachedService.py

@@ -3,7 +3,10 @@
 
 from PyViCare.PyViCareCachedService import ViCareCachedService
 from PyViCare.PyViCareService import ViCareDeviceAccessor
-from PyViCare.PyViCareUtils import PyViCareNotSupportedFeatureError
+from PyViCare.PyViCareUtils import (PyViCareDeviceCommunicationError,
+                                    PyViCareInternalServerError,
+                                    PyViCareInvalidDataError,
+                                    PyViCareNotSupportedFeatureError)
 from tests.helper import now_is
 
 
@@ -67,3 +70,67 @@ def test_setProperty_invalidateCache(self):
 
             self.service.getProperty("someprop")
             self.assertEqual(self.oauth_mock.get.call_count, 2)
+
+    def test_device_communication_error_returns_stale_cache(self):
+        """When device goes offline after successful fetch, return stale cache."""
+        with now_is('2000-01-01 00:00:00'):
+            self.service.getProperty("someprop")
+
+        # Device goes offline after cache expires
+        self.oauth_mock.get.side_effect = PyViCareDeviceCommunicationError(
+            {"errorType": "DEVICE_COMMUNICATION_ERROR",
+             "extendedPayload": {"reason": "GATEWAY_OFFLINE"}})
+
+        with now_is('2000-01-01 00:01:10'):
+            result = self.service.getProperty("someprop")
+
+        self.assertIsNotNone(result)
+
+    def test_server_error_returns_stale_cache(self):
+        """When server returns 500 after successful fetch, return stale cache."""
+        with now_is('2000-01-01 00:00:00'):
+            self.service.getProperty("someprop")
+
+        self.oauth_mock.get.side_effect = PyViCareInternalServerError(
+            {"statusCode": 500, "message": "Internal server error",
+             "viErrorId": "test"})
+
+        with now_is('2000-01-01 00:01:10'):
+            result = self.service.getProperty("someprop")
+
+        self.assertIsNotNone(result)
+
+    def test_device_communication_error_raises_without_cache(self):
+        """When device is offline on first fetch (no cache), must raise."""
+        self.oauth_mock.get.side_effect = PyViCareDeviceCommunicationError(
+            {"errorType": "DEVICE_COMMUNICATION_ERROR",
+             "extendedPayload": {"reason": "DEVICE_OFFLINE"}})
+
+        with now_is('2000-01-01 00:00:00'):
+            self.assertRaises(
+                PyViCareDeviceCommunicationError,
+                self.service.getProperty, "someprop")
+
+    def test_server_error_raises_without_cache(self):
+        """When server errors on first fetch (no cache), must raise."""
+        self.oauth_mock.get.side_effect = PyViCareInternalServerError(
+            {"statusCode": 500, "message": "Internal server error",
+             "viErrorId": "test"})
+
+        with now_is('2000-01-01 00:00:00'):
+            self.assertRaises(
+                PyViCareInternalServerError,
+                self.service.getProperty, "someprop")
+
+    def test_invalid_data_still_raises_with_cache(self):
+        """PyViCareInvalidDataError (genuine bad data) must still raise even with cache."""
+        with now_is('2000-01-01 00:00:00'):
+            self.service.getProperty("someprop")
+
+        self.oauth_mock.get.side_effect = None
+        self.oauth_mock.get.return_value = {"unexpected": "response"}
+
+        with now_is('2000-01-01 00:01:10'):
+            self.assertRaises(
+                PyViCareInvalidDataError,
+                self.service.getProperty, "someprop")

tests/test_PyViCareExceptions.py

@@ -1,7 +1,9 @@
 import datetime
 import unittest
 
-from PyViCare.PyViCareUtils import PyViCareCommandError, PyViCareRateLimitError
+from PyViCare.PyViCareUtils import (PyViCareCommandError,
+                                    PyViCareDeviceCommunicationError,
+                                    PyViCareRateLimitError)
 from tests.helper import readJson
 
 
@@ -18,6 +20,27 @@ def test_createFromResponse(self):
             2020, 3, 17, 16, 20, 10, 106000))
 
 
+class TestPyViCareDeviceCommunicationError(unittest.TestCase):
+
+    def test_createFromGatewayOffline(self):
+        mockResponse = readJson('response/errors/gateway_offline.json')
+
+        error = PyViCareDeviceCommunicationError(mockResponse)
+
+        self.assertEqual(
+            error.message, 'Device communication error. Reason: GATEWAY_OFFLINE')
+        self.assertEqual(error.reason, 'GATEWAY_OFFLINE')
+
+    def test_createFromDeviceOffline(self):
+        mockResponse = readJson('response/errors/device_offline.json')
+
+        error = PyViCareDeviceCommunicationError(mockResponse)
+
+        self.assertEqual(
+            error.message, 'Device communication error. Reason: DEVICE_OFFLINE')
+        self.assertEqual(error.reason, 'DEVICE_OFFLINE')
+
+
 class TestPyViCareCommandError(unittest.TestCase):
 
     def test_createFromResponse(self):

tests/test_ViCareOAuthManager.py

@@ -3,6 +3,7 @@
 
 from PyViCare.PyViCareOAuthManager import AbstractViCareOAuthManager
 from PyViCare.PyViCareUtils import (PyViCareCommandError,
+                                    PyViCareDeviceCommunicationError,
                                     PyViCareInternalServerError,
                                     PyViCareRateLimitError)
 from tests.helper import readJson
@@ -54,6 +55,22 @@ def func():
             return self.manager.get("/")
         self.assertRaises(PyViCareInternalServerError, func)
 
+    def test_get_raisedevicecommunicationerror_gateway_offline(self):
+        self.oauth_mock.get.return_value = FakeResponse(
+            'response/errors/gateway_offline.json')
+
+        def func():
+            return self.manager.get("/")
+        self.assertRaises(PyViCareDeviceCommunicationError, func)
+
+    def test_get_raisedevicecommunicationerror_device_offline(self):
+        self.oauth_mock.get.return_value = FakeResponse(
+            'response/errors/device_offline.json')
+
+        def func():
+            return self.manager.get("/")
+        self.assertRaises(PyViCareDeviceCommunicationError, func)
+
     def test_get_renewtoken_ifexpired(self):
         self.oauth_mock.get.side_effect = [
             FakeResponse('response/errors/expired_token.json'),  # first call expired