Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 8 additions & 49 deletions supervisor/api/proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import aiohttp
from aiohttp import WSCloseCode, WSMessageTypeError, web
from aiohttp.client_exceptions import ClientConnectorError
from aiohttp.client_ws import ClientWebSocketResponse
from aiohttp.hdrs import AUTHORIZATION, CONTENT_TYPE
from aiohttp.http_websocket import WSMsgType
Expand Down Expand Up @@ -179,57 +178,17 @@ async def api(self, request: web.Request):

async def _websocket_client(self) -> ClientWebSocketResponse:
"""Initialize a WebSocket API connection."""
url = f"{self.sys_homeassistant.api_url}/api/websocket"

try:
client = await self.sys_websession.ws_connect(
url, heartbeat=30, ssl=False, max_msg_size=MAX_MESSAGE_SIZE_FROM_CORE
)

# Handle authentication
data = await client.receive_json()

if data.get("type") == "auth_ok":
return client

if data.get("type") != "auth_required":
# Invalid protocol
raise APIError(
f"Got unexpected response from Home Assistant WebSocket: {data}",
_LOGGER.error,
)

# Auth session
await self.sys_homeassistant.api.ensure_access_token()
await client.send_json(
{
"type": "auth",
"access_token": self.sys_homeassistant.api.access_token,
},
dumps=json_dumps,
ws_client = await self.sys_homeassistant.api.connect_websocket(
max_msg_size=MAX_MESSAGE_SIZE_FROM_CORE
)

data = await client.receive_json()

if data.get("type") == "auth_ok":
return client

# Renew the Token is invalid
if (
data.get("type") == "invalid_auth"
and self.sys_homeassistant.refresh_token
):
self.sys_homeassistant.api.access_token = None
return await self._websocket_client()

raise HomeAssistantAuthError()

except (RuntimeError, ValueError, TypeError, ClientConnectorError) as err:
return ws_client.client
except HomeAssistantAPIError as err:
_LOGGER.error("Error connecting to Home Assistant WebSocket: %s", err)
raise APIError() from err
Comment thread
agners marked this conversation as resolved.
Outdated
except (RuntimeError, ValueError, TypeError) as err:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
except (RuntimeError, ValueError, TypeError) as err:
except RuntimeError, ValueError, TypeError as err:

Looks like the AI is still not doing what you want here despite the instructions change. Is this a mypy problem by any chance? When it did it to me I noticed it was driven from a mypy error about it which was really because mypy was out of date in that environment.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be that this code predates my CLAUDE.md file change. That said, I wonder why we need to capture RuntimeError here, that usually indicates a logic error in our code. I wonder if we should remove this from the list 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah with the centralized authentication logic in websocket.py:connect() we capture all exceptions there. Which is probably also not all that nice.

RuntimeError only really seems to be raised if the connection is closed. From what I can tell, this can't really happen as we open a new connection in that case. RuntimeError has been handled in proxy.py since it's inception, its not clear to me if that ever happened in practice.

Let's remove RuntimeError. In case it happens in practice, we'll discover it through Sentry then. I've also got rid of the broad except Exception in connect(), and convert those into HomeAssistantAPIError. I think with that we have quite a bit better exception handling than before.

_LOGGER.error("Client error on WebSocket API %s.", err)
except HomeAssistantAuthError:
_LOGGER.error("Failed authentication to Home Assistant WebSocket")

raise APIError()
raise APIError() from err
Comment thread
agners marked this conversation as resolved.
Outdated

async def _proxy_message(
self,
Expand Down
3 changes: 2 additions & 1 deletion supervisor/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@
FILE_SUFFIX_CONFIGURATION = [".yaml", ".yml", ".json"]

MACHINE_ID = Path("/etc/machine-id")
RUN_SUPERVISOR_STATE = Path("/run/supervisor")
SOCKET_CORE = Path("/run/os/core.sock")
SOCKET_DBUS = Path("/run/dbus/system_bus_socket")
SOCKET_DOCKER = Path("/run/docker.sock")
RUN_SUPERVISOR_STATE = Path("/run/supervisor")
SYSTEMD_JOURNAL_PERSISTENT = Path("/var/log/journal")
SYSTEMD_JOURNAL_VOLATILE = Path("/run/log/journal")

Expand Down
1 change: 1 addition & 0 deletions supervisor/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ async def stop(self) -> None:
self.sys_create_task(coro)
for coro in (
self.sys_websession.close(),
self.sys_homeassistant.api.close(),
self.sys_ingress.unload(),
self.sys_hardware.unload(),
self.sys_dbus.unload(),
Expand Down
10 changes: 10 additions & 0 deletions supervisor/docker/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ class MountBindOptions:

propagation: PropagationMode | None = None
read_only_non_recursive: bool | None = None
create_mountpoint: bool | None = None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem like this is used anywhere?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this came from an earlier iteration, where I used different paths in /run. But we need to have access to that location in Supervisor too, so it must exist at time we use in Supervisor. Hence I don't see any use case for that create mountpoint flag in this PR anymore. I drop it.


def to_dict(self) -> dict[str, Any]:
"""To dictionary representation."""
Expand All @@ -97,6 +98,8 @@ def to_dict(self) -> dict[str, Any]:
out["Propagation"] = self.propagation.value
if self.read_only_non_recursive is not None:
out["ReadOnlyNonRecursive"] = self.read_only_non_recursive
if self.create_mountpoint is not None:
out["CreateMountpoint"] = self.create_mountpoint
return out


Expand Down Expand Up @@ -140,6 +143,7 @@ def to_dict(self) -> dict[str, str | int]:
}


ENV_CORE_API_SOCKET = "SUPERVISOR_CORE_API_SOCKET"
ENV_DUPLICATE_LOG_FILE = "HA_DUPLICATE_LOG_FILE"
ENV_TIME = "TZ"
ENV_TOKEN = "SUPERVISOR_TOKEN"
Expand Down Expand Up @@ -169,6 +173,12 @@ def to_dict(self) -> dict[str, str | int]:
target=MACHINE_ID.as_posix(),
read_only=True,
)
MOUNT_CORE_RUN = DockerMount(
type=MountType.BIND,
source="/run/supervisor",
target="/run/supervisor",
read_only=False,
)
MOUNT_UDEV = DockerMount(
type=MountType.BIND, source="/run/udev", target="/run/udev", read_only=True
)
Expand Down
7 changes: 7 additions & 0 deletions supervisor/docker/homeassistant.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@
from ..jobs.const import JobConcurrency
from ..jobs.decorator import Job
from .const import (
ENV_CORE_API_SOCKET,
ENV_DUPLICATE_LOG_FILE,
ENV_TIME,
ENV_TOKEN,
ENV_TOKEN_OLD,
MOUNT_CORE_RUN,
MOUNT_DBUS,
MOUNT_DEV,
MOUNT_MACHINE_ID,
Expand Down Expand Up @@ -162,6 +164,9 @@ def mounts(self) -> list[DockerMount]:
if self.sys_machine_id:
mounts.append(MOUNT_MACHINE_ID)

if self.sys_homeassistant.api.supports_unix_socket:
mounts.append(MOUNT_CORE_RUN)

return mounts

@Job(
Expand All @@ -180,6 +185,8 @@ async def run(self, *, restore_job_id: str | None = None) -> None:
}
if restore_job_id:
environment[ENV_RESTORE_JOB_ID] = restore_job_id
if self.sys_homeassistant.api.supports_unix_socket:
environment[ENV_CORE_API_SOCKET] = "/run/supervisor/core.sock"
if self.sys_homeassistant.duplicate_log_file:
environment[ENV_DUPLICATE_LOG_FILE] = "1"
await self._run(
Expand Down
5 changes: 5 additions & 0 deletions supervisor/docker/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ def timeout(self) -> int:
def name(self) -> str:
"""Return name of Docker container."""

@property
def attached(self) -> bool:
"""Return True if container/image metadata has been loaded."""
return self._meta is not None

@property
def meta_config(self) -> dict[str, Any]:
"""Return meta data of configuration for container/image."""
Expand Down
Loading
Loading