mirror of
https://github.com/blakeblackshear/frigate.git
synced 2026-01-22 20:18:30 +03:00
fix(recording): handle unexpected filenames in cache maintainer to prevent crash (#21676)
Some checks failed
CI / AMD64 Build (push) Has been cancelled
CI / ARM Build (push) Has been cancelled
CI / Jetson Jetpack 6 (push) Has been cancelled
CI / AMD64 Extra Build (push) Has been cancelled
CI / ARM Extra Build (push) Has been cancelled
CI / Synaptics Build (push) Has been cancelled
CI / Assemble and push default build (push) Has been cancelled
Some checks failed
CI / AMD64 Build (push) Has been cancelled
CI / ARM Build (push) Has been cancelled
CI / Jetson Jetpack 6 (push) Has been cancelled
CI / AMD64 Extra Build (push) Has been cancelled
CI / ARM Extra Build (push) Has been cancelled
CI / Synaptics Build (push) Has been cancelled
CI / Assemble and push default build (push) Has been cancelled
* fix(recording): handle unexpected filenames in cache maintainer to prevent crash * test(recording): add test for maintainer cache file parsing * Prevent log spam from unexpected cache files Addresses PR review feedback: Add deduplication to prevent warning messages from being logged repeatedly for the same unexpected file in the cache directory. Each unexpected filename is only logged once per RecordingMaintainer instance lifecycle. Also adds test to verify warning is only emitted once per filename. * Fix code formatting for test_maintainer.py * fixes + ruff
This commit is contained in:
parent
bf099c3edd
commit
cfeb86646f
@ -97,6 +97,7 @@ class RecordingMaintainer(threading.Thread):
|
|||||||
self.object_recordings_info: dict[str, list] = defaultdict(list)
|
self.object_recordings_info: dict[str, list] = defaultdict(list)
|
||||||
self.audio_recordings_info: dict[str, list] = defaultdict(list)
|
self.audio_recordings_info: dict[str, list] = defaultdict(list)
|
||||||
self.end_time_cache: dict[str, Tuple[datetime.datetime, float]] = {}
|
self.end_time_cache: dict[str, Tuple[datetime.datetime, float]] = {}
|
||||||
|
self.unexpected_cache_files_logged: bool = False
|
||||||
|
|
||||||
async def move_files(self) -> None:
|
async def move_files(self) -> None:
|
||||||
cache_files = [
|
cache_files = [
|
||||||
@ -112,7 +113,14 @@ class RecordingMaintainer(threading.Thread):
|
|||||||
for cache in cache_files:
|
for cache in cache_files:
|
||||||
cache_path = os.path.join(CACHE_DIR, cache)
|
cache_path = os.path.join(CACHE_DIR, cache)
|
||||||
basename = os.path.splitext(cache)[0]
|
basename = os.path.splitext(cache)[0]
|
||||||
|
try:
|
||||||
camera, date = basename.rsplit("@", maxsplit=1)
|
camera, date = basename.rsplit("@", maxsplit=1)
|
||||||
|
except ValueError:
|
||||||
|
if not self.unexpected_cache_files_logged:
|
||||||
|
logger.warning("Skipping unexpected files in cache")
|
||||||
|
self.unexpected_cache_files_logged = True
|
||||||
|
continue
|
||||||
|
|
||||||
start_time = datetime.datetime.strptime(
|
start_time = datetime.datetime.strptime(
|
||||||
date, CACHE_SEGMENT_FORMAT
|
date, CACHE_SEGMENT_FORMAT
|
||||||
).astimezone(datetime.timezone.utc)
|
).astimezone(datetime.timezone.utc)
|
||||||
@ -164,7 +172,13 @@ class RecordingMaintainer(threading.Thread):
|
|||||||
|
|
||||||
cache_path = os.path.join(CACHE_DIR, cache)
|
cache_path = os.path.join(CACHE_DIR, cache)
|
||||||
basename = os.path.splitext(cache)[0]
|
basename = os.path.splitext(cache)[0]
|
||||||
|
try:
|
||||||
camera, date = basename.rsplit("@", maxsplit=1)
|
camera, date = basename.rsplit("@", maxsplit=1)
|
||||||
|
except ValueError:
|
||||||
|
if not self.unexpected_cache_files_logged:
|
||||||
|
logger.warning("Skipping unexpected files in cache")
|
||||||
|
self.unexpected_cache_files_logged = True
|
||||||
|
continue
|
||||||
|
|
||||||
# important that start_time is utc because recordings are stored and compared in utc
|
# important that start_time is utc because recordings are stored and compared in utc
|
||||||
start_time = datetime.datetime.strptime(
|
start_time = datetime.datetime.strptime(
|
||||||
|
|||||||
66
frigate/test/test_maintainer.py
Normal file
66
frigate/test/test_maintainer.py
Normal file
@ -0,0 +1,66 @@
|
|||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
# Mock complex imports before importing maintainer
|
||||||
|
sys.modules["frigate.comms.inter_process"] = MagicMock()
|
||||||
|
sys.modules["frigate.comms.detections_updater"] = MagicMock()
|
||||||
|
sys.modules["frigate.comms.recordings_updater"] = MagicMock()
|
||||||
|
sys.modules["frigate.config.camera.updater"] = MagicMock()
|
||||||
|
|
||||||
|
# Now import the class under test
|
||||||
|
from frigate.config import FrigateConfig # noqa: E402
|
||||||
|
from frigate.record.maintainer import RecordingMaintainer # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
class TestMaintainer(unittest.IsolatedAsyncioTestCase):
|
||||||
|
async def test_move_files_survives_bad_filename(self):
|
||||||
|
config = MagicMock(spec=FrigateConfig)
|
||||||
|
config.cameras = {}
|
||||||
|
stop_event = MagicMock()
|
||||||
|
|
||||||
|
maintainer = RecordingMaintainer(config, stop_event)
|
||||||
|
|
||||||
|
# We need to mock end_time_cache to avoid key errors if logic proceeds
|
||||||
|
maintainer.end_time_cache = {}
|
||||||
|
|
||||||
|
# Mock filesystem
|
||||||
|
# One bad file, one good file
|
||||||
|
files = ["bad_filename.mp4", "camera@20210101000000+0000.mp4"]
|
||||||
|
|
||||||
|
with patch("os.listdir", return_value=files):
|
||||||
|
with patch("os.path.isfile", return_value=True):
|
||||||
|
with patch(
|
||||||
|
"frigate.record.maintainer.psutil.process_iter", return_value=[]
|
||||||
|
):
|
||||||
|
with patch("frigate.record.maintainer.logger.warning") as warn:
|
||||||
|
# Mock validate_and_move_segment to avoid further logic
|
||||||
|
maintainer.validate_and_move_segment = MagicMock()
|
||||||
|
|
||||||
|
try:
|
||||||
|
await maintainer.move_files()
|
||||||
|
except ValueError as e:
|
||||||
|
if "not enough values to unpack" in str(e):
|
||||||
|
self.fail("move_files() crashed on bad filename!")
|
||||||
|
raise e
|
||||||
|
except Exception:
|
||||||
|
# Ignore other errors (like DB connection) as we only care about the unpack crash
|
||||||
|
pass
|
||||||
|
|
||||||
|
# The bad filename is encountered in multiple loops, but should only warn once.
|
||||||
|
matching = [
|
||||||
|
c
|
||||||
|
for c in warn.call_args_list
|
||||||
|
if c.args
|
||||||
|
and isinstance(c.args[0], str)
|
||||||
|
and "Skipping unexpected files in cache" in c.args[0]
|
||||||
|
]
|
||||||
|
self.assertEqual(
|
||||||
|
1,
|
||||||
|
len(matching),
|
||||||
|
f"Expected a single warning for unexpected files, got {len(matching)}",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
Loading…
Reference in New Issue
Block a user