From 64a921254a6c6dc67e5bb59f3706a49cc01fb96d Mon Sep 17 00:00:00 2001 From: Tyson Smith Date: Fri, 27 Sep 2024 17:44:04 -0700 Subject: [PATCH] fix: use .extra files to help prioritize minidump data --- src/ffpuppet/core.py | 7 +------ src/ffpuppet/minidump_parser.py | 26 ++++++++++++++++++++++++++ src/ffpuppet/test_ffpuppet.py | 15 +++++++-------- src/ffpuppet/test_minidump_parser.py | 16 ++++++++++++++++ 4 files changed, 50 insertions(+), 14 deletions(-) diff --git a/src/ffpuppet/core.py b/src/ffpuppet/core.py index eb7a516..e96e59d 100644 --- a/src/ffpuppet/core.py +++ b/src/ffpuppet/core.py @@ -535,12 +535,7 @@ def close(self, force_close: bool = False) -> None: assert self.profile is not None assert self.profile.path is not None assert self._bin_path is not None - # check for minidumps and order by last modified date - # hopefully the oldest log is the cause of the issue - dmp_files = sorted( - (self.profile.path / "minidumps").glob("*.dmp"), - key=lambda x: x.stat().st_mtime, - ) + dmp_files = MinidumpParser.dmp_files(self.profile.path / "minidumps") if dmp_files and not MinidumpParser.mdsw_available(): LOG.error( "Unable to process minidump, minidump-stackwalk is required. %s", diff --git a/src/ffpuppet/minidump_parser.py b/src/ffpuppet/minidump_parser.py index bda6712..c29a7f1 100644 --- a/src/ffpuppet/minidump_parser.py +++ b/src/ffpuppet/minidump_parser.py @@ -196,6 +196,32 @@ def create_log(self, src: Path, filename: str, timeout: int = 300) -> Path: log_fp.write(out_fp.read()) return dst + @staticmethod + def dmp_files(src_dir: Path) -> list[Path]: + """Scan a directory for minidump (.dmp) files. Prioritize files that also have + a MozCrashReason entry in the supporting .extra file. + + Args: + src_dir: Directory containing minidump files. + + Returns: + Dump files. + """ + dmps: list[Path] = [] + for dmp in sorted(src_dir.glob("*.dmp"), key=lambda x: x.stat().st_mtime): + try: + # check .extra file for MozCrashReason entry + with dmp.with_suffix(".extra").open() as out_fp: + has_reason = load(out_fp).get("MozCrashReason") is not None + except (FileNotFoundError, JSONDecodeError): + has_reason = False + # prioritize dmp with MozCrashReason + if has_reason: + dmps.insert(0, dmp) + else: + dmps.append(dmp) + return dmps + @classmethod def mdsw_available(cls, min_version: str = "0.15.2") -> bool: """Check if minidump-stackwalk binary is available. diff --git a/src/ffpuppet/test_ffpuppet.py b/src/ffpuppet/test_ffpuppet.py index e07be9f..7d959d3 100644 --- a/src/ffpuppet/test_ffpuppet.py +++ b/src/ffpuppet/test_ffpuppet.py @@ -24,6 +24,7 @@ BrowserTimeoutError, LaunchError, ) +from .minidump_parser import MinidumpParser from .profile import Profile Bootstrapper.POLL_WAIT = 0.2 @@ -557,11 +558,9 @@ def _fake_create_log(src, filename, _timeout: int = 90): (dst / filename).write_text(src.read_text()) return dst / filename - md_parser = mocker.patch("ffpuppet.core.MinidumpParser") - md_parser.mdsw_available.return_value = mdsw_available - md_parser.return_value.__enter__.return_value.create_log.side_effect = ( - _fake_create_log - ) + mocker.patch("ffpuppet.core.getenv", return_value="1") + mocker.patch.object(MinidumpParser, "mdsw_available", return_value=mdsw_available) + mocker.patch.object(MinidumpParser, "create_log", side_effect=_fake_create_log) profile = tmp_path / "profile" profile.mkdir() (profile / "minidumps").mkdir() @@ -580,14 +579,14 @@ def _fake_create_log(src, filename, _timeout: int = 90): ffp.close() logs = tmp_path / "logs" ffp.save_logs(logs) - assert md_parser.mdsw_available.call_count == 1 if mdsw_available: - assert md_parser.call_count == 1 assert any(logs.glob("log_minidump_00.txt")) assert any(logs.glob("log_minidump_01.txt")) assert any(logs.glob("log_minidump_02.txt")) + assert (logs / "minidumps").is_dir() + assert len(tuple((logs / "minidumps").glob("*"))) == 3 else: - assert md_parser.call_count == 0 + assert not any(logs.glob("log_minidump_*")) def test_ffpuppet_23(tmp_path): diff --git a/src/ffpuppet/test_minidump_parser.py b/src/ffpuppet/test_minidump_parser.py index 851bb7e..5b01b08 100644 --- a/src/ffpuppet/test_minidump_parser.py +++ b/src/ffpuppet/test_minidump_parser.py @@ -273,3 +273,19 @@ def test_minidump_parser_05(mocker, call_result, mdsw_bin, result): mocker.patch("ffpuppet.minidump_parser.run", side_effect=call_result) mocker.patch.object(MinidumpParser, "MDSW_BIN", mdsw_bin) assert MinidumpParser.mdsw_available(min_version="0.15.2") == result + + +def test_minidump_parser_06(tmp_path): + """test MinidumpParser.dmp_files()""" + # empty minidump path + assert not MinidumpParser.dmp_files(tmp_path) + # find single dump file + (tmp_path / "a.dmp").write_text("a") + assert tmp_path / "a.dmp" in MinidumpParser.dmp_files(tmp_path) + # find multiple dump files + (tmp_path / "b.dmp").write_text("b") + (tmp_path / "c.dmp").write_text("c") + assert len(MinidumpParser.dmp_files(tmp_path)) == 3 + # add .extra file to prioritize .dmp file + (tmp_path / "b.extra").write_text('{"MozCrashReason":"foo"}') + assert MinidumpParser.dmp_files(tmp_path)[0] == (tmp_path / "b.dmp")