diff --git a/frigate/api/debug_replay.py b/frigate/api/debug_replay.py index 62307a9f1..ff912b228 100644 --- a/frigate/api/debug_replay.py +++ b/frigate/api/debug_replay.py @@ -38,8 +38,8 @@ class DebugReplayStatusResponse(BaseModel): Returns only session-presence fields. Startup progress and error details flow through the job_state WebSocket topic via the - ``debug_replay`` job (see :mod:`frigate.jobs.debug_replay`); the - Replay page subscribes there with ``useJobStatus("debug_replay")``. + debug_replay job (see frigate.jobs.debug_replay); the + Replay page subscribes there with useJobStatus("debug_replay"). """ active: bool diff --git a/frigate/debug_replay.py b/frigate/debug_replay.py index d2ce576bd..f7e67266b 100644 --- a/frigate/debug_replay.py +++ b/frigate/debug_replay.py @@ -1,8 +1,8 @@ """Debug replay camera management for replaying recordings with detection overlays. The startup work (ffmpeg concat + camera config publish) lives in -``frigate.jobs.debug_replay``. This module owns only session presence -(``active``), session metadata, and post-session cleanup. +frigate.jobs.debug_replay. This module owns only session presence +(active), session metadata, and post-session cleanup. """ import logging @@ -35,9 +35,9 @@ logger = logging.getLogger(__name__) class DebugReplayManager: """Owns the lifecycle pointers for a single debug replay session. - A session exists from the moment ``mark_starting`` is called (synchronously, - inside the API handler) until ``clear_session`` runs (on success cleanup, - failure, or stop). The ``active`` property is the source of truth that the + A session exists from the moment mark_starting is called (synchronously, + inside the API handler) until clear_session runs (on success cleanup, + failure, or stop). The active property is the source of truth that the status bar consumes — broader than the startup job, which only covers the preparing_clip / starting_camera window. """ @@ -52,7 +52,7 @@ class DebugReplayManager: @property def active(self) -> bool: - """True from ``mark_starting`` until ``clear_session``.""" + """True from mark_starting until clear_session.""" return self.replay_camera_name is not None def mark_starting( @@ -64,7 +64,7 @@ class DebugReplayManager: ) -> None: """Synchronously claim the session before the job runner starts. - Called inside the API handler so the status bar sees ``active=True`` + Called inside the API handler so the status bar sees active=True immediately, before the worker thread does any ffmpeg work. """ with self._lock: @@ -82,7 +82,7 @@ class DebugReplayManager: def clear_session(self) -> None: """Reset session pointers without publishing camera removal. - Used by the job runner on failure paths. ``stop()`` does the camera + Used by the job runner on failure paths. stop() does the camera teardown plus this clear in one step. """ with self._lock: @@ -105,7 +105,7 @@ class DebugReplayManager: ) -> None: """Build the in-memory replay camera config and publish the add event. - Called by the job runner during the ``starting_camera`` phase. + Called by the job runner during the starting_camera phase. """ source_config = frigate_config.cameras[source_camera] camera_dict = self._build_camera_config_dict( @@ -121,7 +121,12 @@ class DebugReplayManager: config_data["cameras"] = {} config_data["cameras"][replay_name] = camera_dict - new_config = FrigateConfig.parse_object(config_data) + try: + new_config = FrigateConfig.parse_object(config_data) + except Exception as e: + raise RuntimeError( + f"Failed to validate replay camera config: {e}" + ) from e frigate_config.cameras[replay_name] = new_config.cameras[replay_name] config_publisher.publish_update( diff --git a/frigate/jobs/debug_replay.py b/frigate/jobs/debug_replay.py index c3ea28e0b..832e53306 100644 --- a/frigate/jobs/debug_replay.py +++ b/frigate/jobs/debug_replay.py @@ -1,10 +1,10 @@ """Debug replay startup job: ffmpeg concat + camera config publish. The runner orchestrates the async portion of starting a debug replay -session. The :class:`DebugReplayManager` (in :mod:`frigate.debug_replay`) -owns session presence so the status bar can keep reading a single -``active`` flag from ``/debug_replay/status`` for the entire session -window — which is broader than this job's lifetime. +session. The DebugReplayManager (in frigate.debug_replay) owns session +presence so the status bar can keep reading a single `active` flag from +/debug_replay/status for the entire session window — which is broader +than this job's lifetime. """ import logging @@ -69,8 +69,8 @@ class DebugReplayJob(Job): def to_dict(self) -> dict[str, Any]: """Whitelisted payload for the job_state WS topic. - Replay-specific fields land in ``results`` so the frontend's - generic ``Job`` type can be parameterised cleanly. + Replay-specific fields land in results so the frontend's + generic Job type can be parameterised cleanly. """ return { "id": self.id, @@ -114,8 +114,8 @@ def query_recordings(source_camera: str, start_ts: float, end_ts: float): class DebugReplayJobRunner(threading.Thread): """Worker thread that drives the startup job to completion. - Owns the live ffmpeg ``Popen`` reference for cancellation. Cancellation - is two-step (``threading.Event`` + ``proc.terminate()``) so the runner + Owns the live ffmpeg Popen reference for cancellation. Cancellation + is two-step (threading.Event + proc.terminate()) so the runner both knows it should stop and is unblocked from its blocking subprocess wait. """ @@ -290,10 +290,9 @@ class DebugReplayJobRunner(threading.Thread): self.job.status = JobStatusTypesEnum.cancelled self.job.end_time = time.time() self._broadcast(force=True) - # Manager session pointers are cleared by stop() on the API side - # (it already holds the cleanup contract). On any other cancellation - # path, also clear so /start can run again. - self.replay_manager.clear_session() + # The caller of cancel_debug_replay_job (DebugReplayManager.stop) owns + # session cleanup — db rows, filesystem artifacts, clear_session. We + # only clean up the partial concat output we created. _remove_silent(clip_path) @@ -316,8 +315,8 @@ def start_debug_replay_job( ) -> str: """Validate, create job, start runner. Returns the job id. - Raises ``ValueError`` for bad params (camera missing, time range - invalid, no recordings) and ``RuntimeError`` if a session is already + Raises ValueError for bad params (camera missing, time range + invalid, no recordings) and RuntimeError if a session is already active. """ if job_is_running(JOB_TYPE) or replay_manager.active: diff --git a/frigate/test/test_debug_replay.py b/frigate/test/test_debug_replay.py index 7d04fbd57..f89ae489c 100644 --- a/frigate/test/test_debug_replay.py +++ b/frigate/test/test_debug_replay.py @@ -204,6 +204,43 @@ class TestDebugReplayManagerPublishCamera(unittest.TestCase): topic_arg = publisher.publish_update.call_args.args[0] self.assertEqual(topic_arg.update_type, CameraConfigUpdateEnum.add) + def test_publish_camera_wraps_parse_failure_in_runtime_error(self) -> None: + from frigate.debug_replay import DebugReplayManager + + manager = DebugReplayManager() + frigate_config = MagicMock() + frigate_config.cameras = {"front": MagicMock()} + publisher = MagicMock() + + with ( + patch.object( + manager, + "_build_camera_config_dict", + return_value={"enabled": True}, + ), + patch("frigate.debug_replay.find_config_file", return_value="/cfg.yml"), + patch("frigate.debug_replay.YAML") as yaml_cls, + patch( + "frigate.debug_replay.FrigateConfig.parse_object", + side_effect=ValueError("zone foo has invalid coordinates"), + ), + patch("builtins.open", unittest.mock.mock_open(read_data="cameras:\n")), + ): + yaml_cls.return_value.load.return_value = {"cameras": {}} + + with self.assertRaises(RuntimeError) as ctx: + manager.publish_camera( + source_camera="front", + replay_name="_replay_front", + clip_path="/tmp/clip.mp4", + frigate_config=frigate_config, + config_publisher=publisher, + ) + + self.assertIn("replay camera config", str(ctx.exception)) + self.assertIn("invalid coordinates", str(ctx.exception)) + publisher.publish_update.assert_not_called() + if __name__ == "__main__": unittest.main() diff --git a/frigate/test/test_debug_replay_job.py b/frigate/test/test_debug_replay_job.py index 928807e58..60997564f 100644 --- a/frigate/test/test_debug_replay_job.py +++ b/frigate/test/test_debug_replay_job.py @@ -449,6 +449,11 @@ class TestRunnerCancellation(unittest.TestCase): job = get_current_job("debug_replay") self.assertEqual(job.status, JobStatusTypesEnum.cancelled) + # Runner must not clear the manager session on cancellation — + # that belongs to the caller of cancel_debug_replay_job (stop()). + # If the runner cleared it, stop() would log "no active session" + # and skip its cleanup_db / cleanup_files calls. + self.assertTrue(self.manager.active) if __name__ == "__main__": diff --git a/web/public/locales/en/views/replay.json b/web/public/locales/en/views/replay.json index 73599a086..65711732e 100644 --- a/web/public/locales/en/views/replay.json +++ b/web/public/locales/en/views/replay.json @@ -27,24 +27,24 @@ } }, "page": { - "noSession": "No Active Replay Session", - "noSessionDesc": "Start a debug replay from the History view by clicking the Debug Replay button in the toolbar.", + "noSession": "No Active Debug Replay Session", + "noSessionDesc": "Start a Debug Replay from History view by clicking the Actions button in the toolbar and choosing Debug Replay.", "goToRecordings": "Go to History", - "preparingClip": "Preparing replay clip…", - "preparingClipDesc": "Stitching together recordings for the selected time range. This can take a minute for longer ranges.", - "startingCamera": "Starting replay camera…", + "preparingClip": "Preparing clip…", + "preparingClipDesc": "Frigate is stitching together recordings for the selected time range. This can take a minute for longer ranges.", + "startingCamera": "Starting Debug Replay…", "startError": { - "title": "Failed to start debug replay", + "title": "Failed to start Debug Replay", "back": "Back to History" }, "sourceCamera": "Source Camera", "replayCamera": "Replay Camera", - "initializingReplay": "Initializing replay...", - "stoppingReplay": "Stopping replay...", + "initializingReplay": "Initializing Debug Replay...", + "stoppingReplay": "Stopping Debug Replay...", "stopReplay": "Stop Replay", "confirmStop": { "title": "Stop Debug Replay?", - "description": "This will stop the replay session and clean up all temporary data. Are you sure?", + "description": "This will stop the session and clean up all temporary data. Are you sure?", "confirm": "Stop Replay", "cancel": "Cancel" }, @@ -55,6 +55,6 @@ "activeTracking": "Active tracking", "noActiveTracking": "No active tracking", "configuration": "Configuration", - "configurationDesc": "Fine tune motion detection and object tracking settings for the debug replay camera. No changes are saved to your Frigate configuration file." + "configurationDesc": "Fine tune motion detection and object tracking settings for the Debug Replay camera. No changes are saved to your Frigate configuration file." } } diff --git a/web/src/pages/Replay.tsx b/web/src/pages/Replay.tsx index 698fa7b19..5a574a701 100644 --- a/web/src/pages/Replay.tsx +++ b/web/src/pages/Replay.tsx @@ -177,7 +177,6 @@ export default function Replay() { position: "top-center", }); refreshStatus(); - navigate("/review"); }) .catch((error) => { const errorMessage = @@ -191,7 +190,7 @@ export default function Replay() { .finally(() => { setIsStopping(false); }); - }, [navigate, refreshStatus, t]); + }, [refreshStatus, t]); // Camera activity for the replay camera const { data: config } = useSWR("config", { @@ -278,8 +277,11 @@ export default function Replay() { ); } - // No active session. - if (!status?.active) { + // No active session. Also covers the brief window between the runner + // pushing job.status = "cancelled" via WS and the next SWR refresh + // flipping status.active to false — without this, render falls through + // to the full replay UI and you see a flash of it before stop completes. + if (!status?.active || replayJob?.status === "cancelled") { return (
@@ -320,7 +322,7 @@ export default function Replay() {
) : ( - + )} {phaseTitle}