diff --git a/src/ota_metadata/legacy/parser.py b/src/ota_metadata/legacy/parser.py index d4180102b..9605237b2 100644 --- a/src/ota_metadata/legacy/parser.py +++ b/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}" ) diff --git a/src/ota_proxy/server_app.py b/src/ota_proxy/server_app.py index 0ab4e467c..d1f815b30 100644 --- a/src/ota_proxy/server_app.py +++ b/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 ) diff --git a/src/otaclient/app/ota_client.py b/src/otaclient/app/ota_client.py index ca3af8ce5..4c871ea20 100644 --- a/src/otaclient/app/ota_client.py +++ b/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)