Fix recorder setup hanging if non live schema migration fails (#122242)

homeassistant/components/recorder/core.py

@@ -488,7 +488,7 @@ class Recorder(threading.Thread):
         async_at_started(self.hass, self._async_hass_started)
 
     @callback
-    def _async_startup_failed(self) -> None:
+    def _async_startup_done(self, startup_failed: bool) -> None:
         """Report startup failure."""
         # If a live migration failed, we were able to connect (async_db_connected
         # marked True), the database was marked ready (async_db_ready marked
@@ -499,11 +499,12 @@ class Recorder(threading.Thread):
             self.async_db_connected.set_result(False)
         if not self.async_db_ready.done():
             self.async_db_ready.set_result(False)
-        persistent_notification.async_create(
-            self.hass,
-            "The recorder could not start, check [the logs](/config/logs)",
-            "Recorder",
-        )
+        if startup_failed:
+            persistent_notification.async_create(
+                self.hass,
+                "The recorder could not start, check [the logs](/config/logs)",
+                "Recorder",
+            )
         self._async_stop_listeners()
 
     @callback
@@ -1484,16 +1485,15 @@ class Recorder(threading.Thread):
     def _shutdown(self) -> None:
         """Save end time for current run."""
         _LOGGER.debug("Shutting down recorder")
-        if not self.schema_version or self.schema_version != SCHEMA_VERSION:
-            # If the schema version is not set, we never had a working
-            # connection to the database or the schema never reached a
-            # good state.
-            #
-            # In either case, we want to mark startup as failed.
-            #
-            self.hass.add_job(self._async_startup_failed)
-        else:
-            self.hass.add_job(self._async_stop_listeners)
+
+        # If the schema version is not set, we never had a working
+        # connection to the database or the schema never reached a
+        # good state.
+        # In either case, we want to mark startup as failed.
+        startup_failed = (
+            not self.schema_version or self.schema_version != SCHEMA_VERSION
+        )
+        self.hass.add_job(self._async_startup_done, startup_failed)
 
         try:
             self._end_session()

tests/components/recorder/test_migrate.py

@@ -200,8 +200,14 @@ async def test_database_migration_encounters_corruption(
     assert move_away.called
 
 
+@pytest.mark.parametrize(
+    ("live_migration", "expected_setup_result"), [(True, True), (False, False)]
+)
 async def test_database_migration_encounters_corruption_not_sqlite(
-    hass: HomeAssistant, async_setup_recorder_instance: RecorderInstanceGenerator
+    hass: HomeAssistant,
+    async_setup_recorder_instance: RecorderInstanceGenerator,
+    live_migration: bool,
+    expected_setup_result: bool,
 ) -> None:
     """Test we fail on database error when we cannot recover."""
     assert recorder.util.async_migration_in_progress(hass) is False
@@ -226,8 +232,14 @@ async def test_database_migration_encounters_corruption_not_sqlite(
             "homeassistant.components.persistent_notification.dismiss",
             side_effect=pn.dismiss,
         ) as mock_dismiss,
+        patch(
+            "homeassistant.components.recorder.core.migration.live_migration",
+            return_value=live_migration,
+        ),
     ):
-        await async_setup_recorder_instance(hass, wait_recorder=False)
+        await async_setup_recorder_instance(
+            hass, wait_recorder=False, expected_setup_result=expected_setup_result
+        )
         hass.states.async_set("my.entity", "on", {})
         hass.states.async_set("my.entity", "off", {})
         await hass.async_block_till_done()

tests/conftest.py

@@ -1394,6 +1394,8 @@ async def _async_init_recorder_component(
     hass: HomeAssistant,
     add_config: dict[str, Any] | None = None,
     db_url: str | None = None,
+    *,
+    expected_setup_result: bool,
 ) -> None:
     """Initialize the recorder asynchronously."""
     # pylint: disable-next=import-outside-toplevel
@@ -1408,10 +1410,13 @@ async def _async_init_recorder_component(
     with patch("homeassistant.components.recorder.ALLOW_IN_MEMORY_DB", True):
         if recorder.DOMAIN not in hass.data:
             recorder_helper.async_initialize_recorder(hass)
-        assert await async_setup_component(
-            hass, recorder.DOMAIN, {recorder.DOMAIN: config}
+        setup_task = asyncio.ensure_future(
+            async_setup_component(hass, recorder.DOMAIN, {recorder.DOMAIN: config})
         )
-        assert recorder.DOMAIN in hass.config.components
+        # Wait for recorder integration to setup
+        setup_result = await setup_task
+        assert setup_result == expected_setup_result
+        assert (recorder.DOMAIN in hass.config.components) == expected_setup_result
     _LOGGER.info(
         "Test recorder successfully started, database location: %s",
         config[recorder.CONF_DB_URL],
@@ -1527,10 +1532,16 @@ async def async_test_recorder(
             hass: HomeAssistant,
             config: ConfigType | None = None,
             *,
+            expected_setup_result: bool = True,
             wait_recorder: bool = True,
         ) -> AsyncGenerator[recorder.Recorder]:
             """Setup and return recorder instance."""  # noqa: D401
-            await _async_init_recorder_component(hass, config, recorder_db_url)
+            await _async_init_recorder_component(
+                hass,
+                config,
+                recorder_db_url,
+                expected_setup_result=expected_setup_result,
+            )
             await hass.async_block_till_done()
             instance = hass.data[recorder.DATA_INSTANCE]
             # The recorder's worker is not started until Home Assistant is running
@@ -1557,12 +1568,18 @@ async def async_setup_recorder_instance(
             hass: HomeAssistant,
             config: ConfigType | None = None,
             *,
+            expected_setup_result: bool = True,
             wait_recorder: bool = True,
         ) -> AsyncGenerator[recorder.Recorder]:
             """Set up and return recorder instance."""
 
             return await stack.enter_async_context(
-                async_test_recorder(hass, config, wait_recorder=wait_recorder)
+                async_test_recorder(
+                    hass,
+                    config,
+                    expected_setup_result=expected_setup_result,
+                    wait_recorder=wait_recorder,
+                )
             )
 
         yield async_setup_recorder