Review fix: reopened-record setup closes the database when seeding fails
Review round 5 (Codex gpt-5.6-terra, MEDIUM): the reopened-runtime setup path leaked the just-opened connection if owner lookup or seed_setup_admin raised, so HA's setup retries could accumulate open handles. Seeding on that path is now wrapped symmetrically with the first-setup path: any failure closes the database before re-raising, with a test that fails seeding on a re-setup and asserts the close. Suite: 909 passed.
This commit is contained in:
@@ -270,22 +270,30 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
|
|||||||
db = await _async_open_database(
|
db = await _async_open_database(
|
||||||
hass, await async_get_db_path(hass)
|
hass, await async_get_db_path(hass)
|
||||||
)
|
)
|
||||||
stored_admin_ids = (
|
try:
|
||||||
entry.options.get(CONF_ADMIN_USER_IDS)
|
stored_admin_ids = (
|
||||||
or entry.data.get(CONF_ADMIN_USER_IDS)
|
entry.options.get(CONF_ADMIN_USER_IDS)
|
||||||
)
|
or entry.data.get(CONF_ADMIN_USER_IDS)
|
||||||
context = getattr(entry, "context", None)
|
)
|
||||||
context_user_id = (
|
context = getattr(entry, "context", None)
|
||||||
context.get("user_id")
|
context_user_id = (
|
||||||
if isinstance(context, dict)
|
context.get("user_id")
|
||||||
else None
|
if isinstance(context, dict)
|
||||||
)
|
else None
|
||||||
await seed_setup_admin(
|
)
|
||||||
db,
|
await seed_setup_admin(
|
||||||
stored_admin_ids=stored_admin_ids,
|
db,
|
||||||
context_user_id=context_user_id,
|
stored_admin_ids=stored_admin_ids,
|
||||||
owner_ids=await _async_owner_user_ids(hass),
|
context_user_id=context_user_id,
|
||||||
)
|
owner_ids=await _async_owner_user_ids(hass),
|
||||||
|
)
|
||||||
|
except BaseException:
|
||||||
|
# A seeding failure must not leak the just-opened
|
||||||
|
# connection: close it before the error propagates, so
|
||||||
|
# HA's setup retry starts from a clean handle (the
|
||||||
|
# first-setup path closes symmetrically).
|
||||||
|
await db.close()
|
||||||
|
raise
|
||||||
existing.database = db
|
existing.database = db
|
||||||
entry.runtime_data = existing
|
entry.runtime_data = existing
|
||||||
return True
|
return True
|
||||||
|
|||||||
@@ -375,3 +375,43 @@ def test_seed_owner_ids_argument(tmp_path) -> None:
|
|||||||
return seeded
|
return seeded
|
||||||
|
|
||||||
_with_db(tmp_path, "seed-owner-arg.db")(_body)
|
_with_db(tmp_path, "seed-owner-arg.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_reopened_setup_failure_closes_database(
|
||||||
|
hass, make_entry, monkeypatch
|
||||||
|
) -> None:
|
||||||
|
"""A seeding failure on the reopened-record path closes the just-
|
||||||
|
opened connection before re-raising (no leaked handles across HA's
|
||||||
|
setup retries)."""
|
||||||
|
import pytest as pytest_module
|
||||||
|
from unittest.mock import AsyncMock
|
||||||
|
|
||||||
|
import custom_components.nestquest as nestquest_module
|
||||||
|
from custom_components.nestquest.db import NestQuestDatabase
|
||||||
|
|
||||||
|
# First: a successful setup to create the runtime record and file.
|
||||||
|
entry = await _setup_entry(hass, make_entry(), hass.registry)
|
||||||
|
await async_unload_entry(hass, entry)
|
||||||
|
|
||||||
|
# Now a re-setup whose seeding fails after the reopen.
|
||||||
|
async def _failing_seed(database, **kwargs):
|
||||||
|
raise RuntimeError("seed boom")
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
nestquest_module, "seed_setup_admin", _failing_seed, raising=True
|
||||||
|
)
|
||||||
|
|
||||||
|
close_calls: list[bool] = []
|
||||||
|
original_close = NestQuestDatabase.close
|
||||||
|
|
||||||
|
async def _spy_close(self):
|
||||||
|
close_calls.append(True)
|
||||||
|
return await original_close(self)
|
||||||
|
|
||||||
|
monkeypatch.setattr(NestQuestDatabase, "close", _spy_close)
|
||||||
|
|
||||||
|
with pytest_module.raises(RuntimeError, match="seed boom"):
|
||||||
|
await _setup_entry(hass, entry, hass.registry)
|
||||||
|
# The reopen path opened AND closed a connection; the failure is
|
||||||
|
# the last thing observed.
|
||||||
|
assert close_calls[-1] is True
|
||||||
|
|||||||
Reference in New Issue
Block a user