fix(backport v3.8.x): fix otaclient doesn't interrupt OTA properly when OTA image is invalid(hit HTTPError 403, 404 or 401)

src/ota_metadata/legacy/parser.py

@@ -49,6 +49,7 @@
 from dataclasses import dataclass, fields
 from enum import Enum
 from functools import partial
+from http import HTTPStatus
 from os import PathLike
 from pathlib import Path
 from tempfile import NamedTemporaryFile, TemporaryDirectory
@@ -69,7 +70,9 @@
 )
 from urllib.parse import quote
 
+import requests.exceptions as requests_exc
 from OpenSSL import crypto
+from requests import Response
 from typing_extensions import Self
 
 from ota_proxy import OTAFileCacheControl
@@ -103,6 +106,9 @@ def _python_exit():
 atexit.register(_python_exit)
 
 
+class OTAImageInvalid(Exception): ...
+
+
 class MetadataJWTPayloadInvalid(Exception):
     """Raised when verification passed, but input metadata.jwt is invalid."""
 
@@ -667,6 +673,18 @@ def _process_metadata_jwt(self) -> _MetadataJWTClaimsLayout:
                     )
                     break
                 except Exception as e:
+                    if (
+                        isinstance(e, requests_exc.HTTPError)
+                        and isinstance((_response := e.response), Response)
+                        and _response.status_code
+                        in [
+                            HTTPStatus.FORBIDDEN,
+                            HTTPStatus.UNAUTHORIZED,
+                            HTTPStatus.NOT_FOUND,
+                        ]
+                    ):
+                        raise OTAImageInvalid("failed to download metadata") from e
+
                     logger.warning(
                         f"failed to download {_downloaded_meta_f}, retrying: {e!r}"
                     )

src/ota_proxy/server_app.py

@@ -43,11 +43,11 @@
 from .ota_cache import OTACache
 
 logger = logging.getLogger(__name__)
-connection_err_logger = logging.getLogger(f"{__name__}.connection_err")
-# NOTE: for connection_error, only allow max 6 lines of logging per 30 seconds
-connection_err_logger.addFilter(
+burst_suppressed_logger = logging.getLogger(f"{__name__}.request_error")
+# NOTE: for request_error, only allow max 6 lines of logging per 30 seconds
+burst_suppressed_logger.addFilter(
     BurstSuppressFilter(
-        f"{__name__}.connection_err",
+        f"{__name__}.request_error",
         upper_logger_name=__name__,
         burst_round_length=30,
         burst_max=6,
@@ -211,11 +211,11 @@ async def _error_handling_for_cache_retrieving(self, url: str, send):
             yield _is_succeeded
             _is_succeeded.set()
         except aiohttp.ClientResponseError as e:
-            logger.error(f"{_common_err_msg} due to HTTP error: {e!r}")
+            burst_suppressed_logger.error(f"{_common_err_msg} due to HTTP error: {e!r}")
             # passthrough 4xx(currently 403 and 404) to otaclient
             await self._respond_with_error(e.status, e.message, send)
         except aiohttp.ClientConnectionError as e:
-            connection_err_logger.error(
+            burst_suppressed_logger.error(
                 f"{_common_err_msg} due to connection error: {e!r}"
             )
             await self._respond_with_error(
@@ -224,12 +224,14 @@ async def _error_handling_for_cache_retrieving(self, url: str, send):
                 send,
             )
         except aiohttp.ClientError as e:
-            logger.error(f"{_common_err_msg} due to aiohttp client error: {e!r}")
+            burst_suppressed_logger.error(
+                f"{_common_err_msg} due to aiohttp client error: {e!r}"
+            )
             await self._respond_with_error(
                 HTTPStatus.SERVICE_UNAVAILABLE, f"client error: {e!r}", send
             )
         except (BaseOTACacheError, StopAsyncIteration) as e:
-            logger.error(
+            burst_suppressed_logger.error(
                 f"{_common_err_msg} due to handled ota_cache internal error: {e!r}"
             )
             await self._respond_with_error(
@@ -238,7 +240,7 @@ async def _error_handling_for_cache_retrieving(self, url: str, send):
         except Exception as e:
             # exceptions rather than aiohttp error indicates
             # internal errors of ota_cache
-            logger.exception(
+            burst_suppressed_logger.exception(
                 f"{_common_err_msg} due to unhandled ota_cache internal error: {e!r}"
             )
             await self._respond_with_error(
@@ -255,13 +257,13 @@ async def _error_handling_during_transferring(self, url: str, send):
         try:
             yield
         except (BaseOTACacheError, StopAsyncIteration) as e:
-            logger.error(
+            burst_suppressed_logger.error(
                 f"{_common_err_msg=} due to handled ota_cache internal error: {e!r}"
             )
             await self._send_chunk(b"", False, send)
         except Exception as e:
             # unexpected internal errors of ota_cache
-            logger.exception(
+            burst_suppressed_logger.error(
                 f"{_common_err_msg=} due to unhandled ota_cache internal error: {e!r}"
             )
             await self._send_chunk(b"", False, send)
@@ -292,7 +294,7 @@ async def _pull_data_and_send(self, url: str, scope, send):
             # retrieve_file executed successfully, but return nothing
             if _is_succeeded.is_set():
                 _msg = f"failed to retrieve fd for {url} from otacache"
-                logger.warning(_msg)
+                burst_suppressed_logger.warning(_msg)
                 await self._respond_with_error(
                     HTTPStatus.INTERNAL_SERVER_ERROR, _msg, send
                 )

src/otaclient/app/ota_client.py

@@ -33,6 +33,7 @@
 from urllib.parse import urlparse
 
 import requests.exceptions as requests_exc
+from requests import Response
 
 from ota_metadata.legacy import parser as ota_metadata_parser
 from ota_metadata.legacy import types as ota_metadata_types
@@ -135,8 +136,13 @@ def _download_exception_handler(_fut: Future[Any]) -> bool:
     try:
         # exceptions that cannot be handled by us
         if isinstance(exc, requests_exc.HTTPError):
-            http_errcode = exc.errno
+            _response = exc.response
+            # NOTE(20241129): if somehow HTTPError doesn't contain response,
+            #       don't do anything but let upper retry.
+            if not isinstance(_response, Response):
+                return False
 
+            http_errcode = _response.status_code
             if http_errcode in [
                 HTTPStatus.FORBIDDEN,
                 HTTPStatus.UNAUTHORIZED,
@@ -413,6 +419,10 @@ def _execute_update(self):
             _err_msg = f"metadata.jwt is invalid: {e!r}"
             logger.error(_err_msg)
             raise ota_errors.MetadataJWTInvalid(_err_msg, module=__name__) from e
+        except ota_metadata_parser.OTAImageInvalid as e:
+            _err_msg = f"OTA image is invalid: {e!r}"
+            logger.error(_err_msg)
+            raise ota_errors.OTAImageInvalid(_err_msg, module=__name__) from e
         except Exception as e:
             _err_msg = f"failed to prepare ota metafiles: {e!r}"
             logger.error(_err_msg)