From 4adb001d488d8e1e46d998074bcfa91611f641c3 Mon Sep 17 00:00:00 2001 From: Joe Becher Date: Wed, 6 May 2026 08:15:28 -0400 Subject: [PATCH 1/2] fix(bundle-analysis): clean up temp files on load failure Co-Authored-By: Claude Sonnet 4.6 --- libs/shared/shared/bundle_analysis/storage.py | 14 +++-- .../bundle_analysis/test_bundle_analysis.py | 53 ++++++++++++++++++- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/libs/shared/shared/bundle_analysis/storage.py b/libs/shared/shared/bundle_analysis/storage.py index 8e2fda69ef..5d4d4ba5e2 100644 --- a/libs/shared/shared/bundle_analysis/storage.py +++ b/libs/shared/shared/bundle_analysis/storage.py @@ -1,4 +1,5 @@ import logging +import os import tempfile from enum import Enum @@ -48,12 +49,15 @@ def load(self, report_key: str) -> BundleAnalysisReport | None: repo_key=self.repo_key, report_key=report_key ) _, db_path = tempfile.mkstemp(prefix="bundle_analysis_") - - with open(db_path, "w+b") as f: - try: + try: + with open(db_path, "w+b") as f: self.storage_service.read_file(self.bucket_name, path, file_obj=f) - except FileNotInStorageError: - return None + except FileNotInStorageError: + os.unlink(db_path) + return None + except Exception: + os.unlink(db_path) + raise return BundleAnalysisReport(db_path) @sentry_sdk.trace diff --git a/libs/shared/tests/unit/bundle_analysis/test_bundle_analysis.py b/libs/shared/tests/unit/bundle_analysis/test_bundle_analysis.py index b46ee59554..df9a36b696 100644 --- a/libs/shared/tests/unit/bundle_analysis/test_bundle_analysis.py +++ b/libs/shared/tests/unit/bundle_analysis/test_bundle_analysis.py @@ -1,3 +1,4 @@ +import os import tempfile from pathlib import Path from unittest import TestCase @@ -21,7 +22,7 @@ Session, get_db_session, ) -from shared.storage.exceptions import PutRequestRateLimitError +from shared.storage.exceptions import FileNotInStorageError, PutRequestRateLimitError sample_bundle_stats_path = ( Path(__file__).parent.parent.parent / "samples" / "sample_bundle_stats.json" @@ -435,6 +436,56 @@ def test_save_load_bundle_report(mock_storage): report.cleanup() +def test_load_cleans_up_tempfile_on_file_not_found(mock_storage): + loader = BundleAnalysisReportLoader(None) + captured_path = [] + + real_mkstemp = tempfile.mkstemp + + def capturing_mkstemp(*args, **kwargs): + fd, path = real_mkstemp(*args, **kwargs) + captured_path.append(path) + return fd, path + + with patch("tempfile.mkstemp", side_effect=capturing_mkstemp): + with patch( + "shared.storage.memory.MemoryStorageService.read_file", + side_effect=FileNotInStorageError(), + ): + result = loader.load("nonexistent-key") + + assert result is None + assert len(captured_path) == 1 + assert not os.path.exists(captured_path[0]), ( + f"Temp file not cleaned up: {captured_path[0]}" + ) + + +def test_load_cleans_up_tempfile_on_unexpected_error(mock_storage): + loader = BundleAnalysisReportLoader(None) + captured_path = [] + + real_mkstemp = tempfile.mkstemp + + def capturing_mkstemp(*args, **kwargs): + fd, path = real_mkstemp(*args, **kwargs) + captured_path.append(path) + return fd, path + + with patch("tempfile.mkstemp", side_effect=capturing_mkstemp): + with patch( + "shared.storage.memory.MemoryStorageService.read_file", + side_effect=RuntimeError("storage exploded"), + ): + with pytest.raises(RuntimeError, match="storage exploded"): + loader.load("some-key") + + assert len(captured_path) == 1 + assert not os.path.exists(captured_path[0]), ( + f"Temp file not cleaned up: {captured_path[0]}" + ) + + def test_reupload_bundle_report(): try: report = BundleAnalysisReport() From f2003b306a88380342bb5b0bba354eca7d059ee7 Mon Sep 17 00:00:00 2001 From: Joe Becher Date: Wed, 6 May 2026 09:58:36 -0400 Subject: [PATCH 2/2] fix(bundle-analysis): close fd leaked by mkstemp on error paths Co-Authored-By: Claude Sonnet 4.6 --- libs/shared/shared/bundle_analysis/storage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libs/shared/shared/bundle_analysis/storage.py b/libs/shared/shared/bundle_analysis/storage.py index 5d4d4ba5e2..fd2ea93f52 100644 --- a/libs/shared/shared/bundle_analysis/storage.py +++ b/libs/shared/shared/bundle_analysis/storage.py @@ -48,7 +48,8 @@ def load(self, report_key: str) -> BundleAnalysisReport | None: path = StoragePaths.bundle_report.path( repo_key=self.repo_key, report_key=report_key ) - _, db_path = tempfile.mkstemp(prefix="bundle_analysis_") + fd, db_path = tempfile.mkstemp(prefix="bundle_analysis_") + os.close(fd) try: with open(db_path, "w+b") as f: self.storage_service.read_file(self.bucket_name, path, file_obj=f)