Skip to content

Commit

Permalink
Move read_text to executor (#5688)
Browse files Browse the repository at this point in the history
* Move read_text to executor

* Fix issues found by coderabbit

* formated to formatted

* switch to async_capture_exception

* Find and replace got one too many

* Update patch mock to async_capture_exception

* Drop Sentry capture from format_message

The error handling got introduced in #2052, however, #2100 essentially
makes sure there will never be a byte object passed to this function.
And even if, the Sentry aiohttp plug-in will properly catch such an
exception.

---------

Co-authored-by: Stefan Agner <[email protected]>
  • Loading branch information
mdegat01 and agners authored Mar 1, 2025
1 parent 12c951f commit 86133f8
Show file tree
Hide file tree
Showing 47 changed files with 205 additions and 167 deletions.
2 changes: 1 addition & 1 deletion supervisor/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def run_os_startup_check_cleanup() -> None:
loop.set_debug(coresys.config.debug)
loop.run_until_complete(coresys.core.connect())

bootstrap.supervisor_debugger(coresys)
loop.run_until_complete(bootstrap.supervisor_debugger(coresys))

# Signal health startup for container
run_os_startup_check_cleanup()
Expand Down
4 changes: 2 additions & 2 deletions supervisor/addons/addon.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
from ..utils import check_port
from ..utils.apparmor import adjust_profile
from ..utils.json import read_json_file, write_json_file
from ..utils.sentry import capture_exception
from ..utils.sentry import async_capture_exception
from .const import (
WATCHDOG_MAX_ATTEMPTS,
WATCHDOG_RETRY_SECONDS,
Expand Down Expand Up @@ -1530,7 +1530,7 @@ async def _restart_after_problem(self, state: ContainerState):
except AddonsError as err:
attempts = attempts + 1
_LOGGER.error("Watchdog restart of addon %s failed!", self.name)
capture_exception(err)
await async_capture_exception(err)
else:
break

Expand Down
6 changes: 3 additions & 3 deletions supervisor/addons/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from ..jobs.decorator import Job, JobCondition
from ..resolution.const import ContextType, IssueType, SuggestionType
from ..store.addon import AddonStore
from ..utils.sentry import capture_exception
from ..utils.sentry import async_capture_exception
from .addon import Addon
from .const import ADDON_UPDATE_CONDITIONS
from .data import AddonsData
Expand Down Expand Up @@ -170,7 +170,7 @@ async def shutdown(self, stage: AddonStartup) -> None:
await addon.stop()
except Exception as err: # pylint: disable=broad-except
_LOGGER.warning("Can't stop Add-on %s: %s", addon.slug, err)
capture_exception(err)
await async_capture_exception(err)

@Job(
name="addon_manager_install",
Expand Down Expand Up @@ -388,7 +388,7 @@ async def sync_dns(self) -> None:
reference=addon.slug,
suggestions=[SuggestionType.EXECUTE_REPAIR],
)
capture_exception(err)
await async_capture_exception(err)
else:
add_host_coros.append(
self.sys_plugins.dns.add_host(
Expand Down
27 changes: 15 additions & 12 deletions supervisor/addons/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,18 +210,6 @@ def description(self) -> str:
"""Return description of add-on."""
return self.data[ATTR_DESCRIPTON]

@property
def long_description(self) -> str | None:
"""Return README.md as long_description."""
readme = Path(self.path_location, "README.md")

# If readme not exists
if not readme.exists():
return None

# Return data
return readme.read_text(encoding="utf-8")

@property
def repository(self) -> str:
"""Return repository of add-on."""
Expand Down Expand Up @@ -646,6 +634,21 @@ def breaking_versions(self) -> list[AwesomeVersion]:
"""Return breaking versions of addon."""
return self.data[ATTR_BREAKING_VERSIONS]

async def long_description(self) -> str | None:
"""Return README.md as long_description."""

def read_readme() -> str | None:
readme = Path(self.path_location, "README.md")

# If readme not exists
if not readme.exists():
return None

# Return data
return readme.read_text(encoding="utf-8")

return await self.sys_run_in_executor(read_readme)

def refresh_path_cache(self) -> Awaitable[None]:
"""Refresh cache of existing paths."""

Expand Down
4 changes: 2 additions & 2 deletions supervisor/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from ..const import AddonState
from ..coresys import CoreSys, CoreSysAttributes
from ..exceptions import APIAddonNotInstalled, HostNotSupportedError
from ..utils.sentry import capture_exception
from ..utils.sentry import async_capture_exception
from .addons import APIAddons
from .audio import APIAudio
from .auth import APIAuth
Expand Down Expand Up @@ -412,7 +412,7 @@ async def get_supervisor_logs(*args, **kwargs):
if not isinstance(err, HostNotSupportedError):
# No need to capture HostNotSupportedError to Sentry, the cause
# is known and reported to the user using the resolution center.
capture_exception(err)
await async_capture_exception(err)
kwargs.pop("follow", None) # Follow is not supported for Docker logs
return await api_supervisor.logs(*args, **kwargs)

Expand Down
2 changes: 1 addition & 1 deletion supervisor/api/addons.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ async def info(self, request: web.Request) -> dict[str, Any]:
ATTR_HOSTNAME: addon.hostname,
ATTR_DNS: addon.dns,
ATTR_DESCRIPTON: addon.description,
ATTR_LONG_DESCRIPTION: addon.long_description,
ATTR_LONG_DESCRIPTION: await addon.long_description(),
ATTR_ADVANCED: addon.advanced,
ATTR_STAGE: addon.stage,
ATTR_REPOSITORY: addon.repository,
Expand Down
8 changes: 4 additions & 4 deletions supervisor/api/host.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ async def info(self, request):
ATTR_VIRTUALIZATION: self.sys_host.info.virtualization,
ATTR_CPE: self.sys_host.info.cpe,
ATTR_DEPLOYMENT: self.sys_host.info.deployment,
ATTR_DISK_FREE: self.sys_host.info.free_space,
ATTR_DISK_TOTAL: self.sys_host.info.total_space,
ATTR_DISK_USED: self.sys_host.info.used_space,
ATTR_DISK_LIFE_TIME: self.sys_host.info.disk_life_time,
ATTR_DISK_FREE: await self.sys_host.info.free_space(),
ATTR_DISK_TOTAL: await self.sys_host.info.total_space(),
ATTR_DISK_USED: await self.sys_host.info.used_space(),
ATTR_DISK_LIFE_TIME: await self.sys_host.info.disk_life_time(),
ATTR_FEATURES: self.sys_host.features,
ATTR_HOSTNAME: self.sys_host.info.hostname,
ATTR_LLMNR_HOSTNAME: self.sys_host.info.llmnr_hostname,
Expand Down
34 changes: 19 additions & 15 deletions supervisor/api/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@
)


def _read_static_file(path: Path) -> Any:
def _read_static_file(path: Path, binary: bool = False) -> Any:
"""Read in a static file asset for API output.
Must be run in executor.
"""
with path.open("r") as asset:
with path.open("rb" if binary else "r") as asset:
return asset.read()


Expand Down Expand Up @@ -109,7 +109,7 @@ def _extract_repository(self, request: web.Request) -> Repository:

return self.sys_store.get(repository_slug)

def _generate_addon_information(
async def _generate_addon_information(
self, addon: AddonStore, extended: bool = False
) -> dict[str, Any]:
"""Generate addon information."""
Expand Down Expand Up @@ -156,7 +156,7 @@ def _generate_addon_information(
ATTR_HOST_NETWORK: addon.host_network,
ATTR_HOST_PID: addon.host_pid,
ATTR_INGRESS: addon.with_ingress,
ATTR_LONG_DESCRIPTION: addon.long_description,
ATTR_LONG_DESCRIPTION: await addon.long_description(),
ATTR_RATING: rating_security(addon),
ATTR_SIGNED: addon.signed,
}
Expand Down Expand Up @@ -185,10 +185,12 @@ async def reload(self, request: web.Request) -> None:
async def store_info(self, request: web.Request) -> dict[str, Any]:
"""Return store information."""
return {
ATTR_ADDONS: [
self._generate_addon_information(self.sys_addons.store[addon])
for addon in self.sys_addons.store
],
ATTR_ADDONS: await asyncio.gather(
*[
self._generate_addon_information(self.sys_addons.store[addon])
for addon in self.sys_addons.store
]
),
ATTR_REPOSITORIES: [
self._generate_repository_information(repository)
for repository in self.sys_store.all
Expand All @@ -199,10 +201,12 @@ async def store_info(self, request: web.Request) -> dict[str, Any]:
async def addons_list(self, request: web.Request) -> dict[str, Any]:
"""Return all store add-ons."""
return {
ATTR_ADDONS: [
self._generate_addon_information(self.sys_addons.store[addon])
for addon in self.sys_addons.store
]
ATTR_ADDONS: await asyncio.gather(
*[
self._generate_addon_information(self.sys_addons.store[addon])
for addon in self.sys_addons.store
]
)
}

@api_process
Expand Down Expand Up @@ -234,7 +238,7 @@ async def addons_addon_info(self, request: web.Request) -> dict[str, Any]:
async def addons_addon_info_wrapped(self, request: web.Request) -> dict[str, Any]:
"""Return add-on information directly (not api)."""
addon: AddonStore = self._extract_addon(request)
return self._generate_addon_information(addon, True)
return await self._generate_addon_information(addon, True)

@api_process_raw(CONTENT_TYPE_PNG)
async def addons_addon_icon(self, request: web.Request) -> bytes:
Expand All @@ -243,7 +247,7 @@ async def addons_addon_icon(self, request: web.Request) -> bytes:
if not addon.with_icon:
raise APIError(f"No icon found for add-on {addon.slug}!")

return await self.sys_run_in_executor(_read_static_file, addon.path_icon)
return await self.sys_run_in_executor(_read_static_file, addon.path_icon, True)

@api_process_raw(CONTENT_TYPE_PNG)
async def addons_addon_logo(self, request: web.Request) -> bytes:
Expand All @@ -252,7 +256,7 @@ async def addons_addon_logo(self, request: web.Request) -> bytes:
if not addon.with_logo:
raise APIError(f"No logo found for add-on {addon.slug}!")

return await self.sys_run_in_executor(_read_static_file, addon.path_logo)
return await self.sys_run_in_executor(_read_static_file, addon.path_logo, True)

@api_process_raw(CONTENT_TYPE_TEXT)
async def addons_addon_changelog(self, request: web.Request) -> str:
Expand Down
6 changes: 3 additions & 3 deletions supervisor/backups/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
from ..utils.common import FileConfiguration
from ..utils.dt import utcnow
from ..utils.sentinel import DEFAULT
from ..utils.sentry import capture_exception
from ..utils.sentry import async_capture_exception
from .backup import Backup
from .const import (
DEFAULT_FREEZE_TIMEOUT,
Expand Down Expand Up @@ -525,7 +525,7 @@ async def _do_backup(
return None
except Exception as err: # pylint: disable=broad-except
_LOGGER.exception("Backup %s error", backup.slug)
capture_exception(err)
await async_capture_exception(err)
self.sys_jobs.current.capture_error(
BackupError(f"Backup {backup.slug} error, see supervisor logs")
)
Expand Down Expand Up @@ -718,7 +718,7 @@ async def _do_restore(
raise
except Exception as err: # pylint: disable=broad-except
_LOGGER.exception("Restore %s error", backup.slug)
capture_exception(err)
await async_capture_exception(err)
raise BackupError(
f"Restore {backup.slug} error, see supervisor logs"
) from err
Expand Down
7 changes: 4 additions & 3 deletions supervisor/bootstrap.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Bootstrap Supervisor."""

# ruff: noqa: T100
from importlib import import_module
import logging
import os
import signal
Expand Down Expand Up @@ -306,12 +307,12 @@ def reg_signal(loop, coresys: CoreSys) -> None:
_LOGGER.warning("Could not bind to SIGINT")


def supervisor_debugger(coresys: CoreSys) -> None:
async def supervisor_debugger(coresys: CoreSys) -> None:
"""Start debugger if needed."""
if not coresys.config.debug:
return
# pylint: disable=import-outside-toplevel
import debugpy

debugpy = await coresys.run_in_executor(import_module, "debugpy")

_LOGGER.info("Initializing Supervisor debugger")

Expand Down
10 changes: 5 additions & 5 deletions supervisor/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from .homeassistant.core import LANDINGPAGE
from .resolution.const import ContextType, IssueType, SuggestionType, UnhealthyReason
from .utils.dt import utcnow
from .utils.sentry import capture_exception
from .utils.sentry import async_capture_exception
from .utils.whoami import WhoamiData, retrieve_whoami

_LOGGER: logging.Logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -172,7 +172,7 @@ async def setup(self):
"Fatal error happening on load Task %s: %s", setup_task, err
)
self.sys_resolution.unhealthy = UnhealthyReason.SETUP
capture_exception(err)
await async_capture_exception(err)

# Set OS Agent diagnostics if needed
if (
Expand All @@ -189,7 +189,7 @@ async def setup(self):
self.sys_config.diagnostics,
err,
)
capture_exception(err)
await async_capture_exception(err)

# Evaluate the system
await self.sys_resolution.evaluate.evaluate_system()
Expand Down Expand Up @@ -246,12 +246,12 @@ async def start(self):
await self.sys_homeassistant.core.start()
except HomeAssistantCrashError as err:
_LOGGER.error("Can't start Home Assistant Core - rebuiling")
capture_exception(err)
await async_capture_exception(err)

with suppress(HomeAssistantError):
await self.sys_homeassistant.core.rebuild()
except HomeAssistantError as err:
capture_exception(err)
await async_capture_exception(err)
else:
_LOGGER.info("Skipping start of Home Assistant")

Expand Down
6 changes: 3 additions & 3 deletions supervisor/dbus/network/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
HostNotSupportedError,
NetworkInterfaceNotFound,
)
from ...utils.sentry import capture_exception
from ...utils.sentry import async_capture_exception
from ..const import (
DBUS_ATTR_CONNECTION_ENABLED,
DBUS_ATTR_DEVICES,
Expand Down Expand Up @@ -223,13 +223,13 @@ async def update(self, changed: dict[str, Any] | None = None) -> None:
device,
err,
)
capture_exception(err)
await async_capture_exception(err)
return
except Exception as err: # pylint: disable=broad-except
_LOGGER.exception(
"Unkown error while processing %s: %s", device, err
)
capture_exception(err)
await async_capture_exception(err)
continue

# Skeep interface
Expand Down
6 changes: 3 additions & 3 deletions supervisor/docker/addon.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
from ..jobs.const import JobCondition, JobExecutionLimit
from ..jobs.decorator import Job
from ..resolution.const import CGROUP_V2_VERSION, ContextType, IssueType, SuggestionType
from ..utils.sentry import capture_exception
from ..utils.sentry import async_capture_exception
from .const import (
ENV_TIME,
ENV_TOKEN,
Expand Down Expand Up @@ -606,7 +606,7 @@ async def run(self) -> None:
)
except CoreDNSError as err:
_LOGGER.warning("Can't update DNS for %s", self.name)
capture_exception(err)
await async_capture_exception(err)

# Hardware Access
if self.addon.static_devices:
Expand Down Expand Up @@ -787,7 +787,7 @@ async def stop(self, remove_container: bool = True) -> None:
await self.sys_plugins.dns.delete_host(self.addon.hostname)
except CoreDNSError as err:
_LOGGER.warning("Can't update DNS for %s", self.name)
capture_exception(err)
await async_capture_exception(err)

# Hardware
if self._hw_listener:
Expand Down
Loading

0 comments on commit 86133f8

Please sign in to comment.