diff --git a/libs/shared/shared/bundle_analysis/storage.py b/libs/shared/shared/bundle_analysis/storage.py index 8e2fda69ef..fd2ea93f52 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 @@ -47,13 +48,17 @@ 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_") - - with open(db_path, "w+b") as f: - try: + 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) - 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()