fix: use .extra files to help prioritize minidump data

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",

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 (src_dir / f"{dmp.stem}.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.

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):

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")