From 51a5248d88d1c62899d39043488c418a09758d95 Mon Sep 17 00:00:00 2001 From: Thomas Farstrike Date: Wed, 24 Dec 2025 11:11:49 +0100 Subject: [PATCH] OSUpdate app: work towards fixing auto-resume --- .../assets/appstore.py | 44 +++++++++++++------ .../assets/osupdate.py | 3 +- .../lib/mpos/net/download_manager.py | 14 +++--- tests/test_download_manager.py | 42 ++++++++---------- 4 files changed, 59 insertions(+), 44 deletions(-) diff --git a/internal_filesystem/builtin/apps/com.micropythonos.appstore/assets/appstore.py b/internal_filesystem/builtin/apps/com.micropythonos.appstore/assets/appstore.py index 0461d4f5..52ad5555 100644 --- a/internal_filesystem/builtin/apps/com.micropythonos.appstore/assets/appstore.py +++ b/internal_filesystem/builtin/apps/com.micropythonos.appstore/assets/appstore.py @@ -60,9 +60,11 @@ class AppStore(Activity): TaskManager.create_task(self.download_app_index(self.app_index_url_github)) async def download_app_index(self, json_url): - response = await DownloadManager.download_url(json_url) - if not response: - self.please_wait_label.set_text(f"Could not download app index from\n{json_url}") + try: + response = await DownloadManager.download_url(json_url) + except Exception as e: + print(f"Failed to download app index: {e}") + self.please_wait_label.set_text(f"Could not download app index from\n{json_url}\nError: {e}") return print(f"Got response text: {response[0:20]}") try: @@ -197,9 +199,10 @@ class AppStore(Activity): async def fetch_badgehub_app_details(self, app_obj): details_url = self.app_detail_url_badgehub + "/" + app_obj.fullname - response = await DownloadManager.download_url(details_url) - if not response: - print(f"Could not download app details from from\n{details_url}") + try: + response = await DownloadManager.download_url(details_url) + except Exception as e: + print(f"Could not download app details from {details_url}: {e}") return print(f"Got response text: {response[0:20]}") try: @@ -480,14 +483,27 @@ class AppDetail(Activity): pass temp_zip_path = "tmp/temp.mpk" print(f"Downloading .mpk file from: {zip_url} to {temp_zip_path}") - result = await DownloadManager.download_url(zip_url, outfile=temp_zip_path, total_size=download_url_size, progress_callback=self.pcb) - if result is not True: - print("Download failed...") # Would be good to show an error to the user if this failed... - else: - print("Downloaded .mpk file, size:", os.stat(temp_zip_path)[6], "bytes") - # Install it: - PackageManager.install_mpk(temp_zip_path, dest_folder) # 60 until 90 percent is the unzip but no progress there... - self.progress_bar.set_value(90, True) + try: + result = await DownloadManager.download_url(zip_url, outfile=temp_zip_path, total_size=download_url_size, progress_callback=self.pcb) + if result is not True: + print("Download failed...") # Would be good to show an error to the user if this failed... + else: + print("Downloaded .mpk file, size:", os.stat(temp_zip_path)[6], "bytes") + # Install it: + PackageManager.install_mpk(temp_zip_path, dest_folder) # 60 until 90 percent is the unzip but no progress there... + self.progress_bar.set_value(90, True) + except Exception as e: + print(f"Download failed with exception: {e}") + self.install_label.set_text(f"Download failed") + self.install_button.remove_state(lv.STATE.DISABLED) + self.progress_bar.add_flag(lv.obj.FLAG.HIDDEN) + self.progress_bar.set_value(0, False) + # Make sure there's no leftover file filling the storage: + try: + os.remove(temp_zip_path) + except Exception: + pass + return # Make sure there's no leftover file filling the storage: try: os.remove(temp_zip_path) diff --git a/internal_filesystem/builtin/apps/com.micropythonos.osupdate/assets/osupdate.py b/internal_filesystem/builtin/apps/com.micropythonos.osupdate/assets/osupdate.py index 07d40c57..14a9af91 100644 --- a/internal_filesystem/builtin/apps/com.micropythonos.osupdate/assets/osupdate.py +++ b/internal_filesystem/builtin/apps/com.micropythonos.osupdate/assets/osupdate.py @@ -482,7 +482,8 @@ class UpdateDownloader: '-113', '-104', '-110', '-118', # Error codes 'econnaborted', 'econnreset', 'etimedout', 'ehostunreach', # Error names 'connection reset', 'connection aborted', # Error messages - 'broken pipe', 'network unreachable', 'host unreachable' + 'broken pipe', 'network unreachable', 'host unreachable', + 'failed to download chunk' # From download_manager OSError(-110) ] return any(indicator in error_str or indicator in error_repr diff --git a/internal_filesystem/lib/mpos/net/download_manager.py b/internal_filesystem/lib/mpos/net/download_manager.py index ed9db2a6..7e4a0bb6 100644 --- a/internal_filesystem/lib/mpos/net/download_manager.py +++ b/internal_filesystem/lib/mpos/net/download_manager.py @@ -200,10 +200,14 @@ async def download_url(url, outfile=None, total_size=None, Returns: bytes: Downloaded content (if outfile and chunk_callback are None) - bool: True if successful, False if failed (when using outfile or chunk_callback) + bool: True if successful (when using outfile or chunk_callback) Raises: + ImportError: If aiohttp module is not available + RuntimeError: If HTTP request fails (status code < 200 or >= 400) + OSError: If chunk download times out after retries or network connection is lost ValueError: If both outfile and chunk_callback are provided + Exception: Other download errors (propagated from aiohttp or chunk processing) Example: # Download to memory @@ -247,7 +251,7 @@ async def download_url(url, outfile=None, total_size=None, session = _get_session() if session is None: print("DownloadManager: Cannot download, aiohttp not available") - return False if (outfile or chunk_callback) else None + raise ImportError("aiohttp module not available") # Increment refcount global _session_refcount @@ -268,7 +272,7 @@ async def download_url(url, outfile=None, total_size=None, async with session.get(url, headers=headers) as response: if response.status < 200 or response.status >= 400: print(f"DownloadManager: HTTP error {response.status}") - return False if (outfile or chunk_callback) else None + raise RuntimeError(f"HTTP {response.status}") # Figure out total size print("DownloadManager: Response headers:", response.headers) @@ -332,7 +336,7 @@ async def download_url(url, outfile=None, total_size=None, print("DownloadManager: ERROR: failed to download chunk after retries!") if fd: fd.close() - return False if (outfile or chunk_callback) else None + raise OSError(-110, "Failed to download chunk after retries") if chunk_data: # Output chunk @@ -384,7 +388,7 @@ async def download_url(url, outfile=None, total_size=None, print(f"DownloadManager: Exception during download: {e}") if fd: fd.close() - return False if (outfile or chunk_callback) else None + raise # Re-raise the exception instead of suppressing it finally: # Decrement refcount if _session_lock: diff --git a/tests/test_download_manager.py b/tests/test_download_manager.py index 0eee1410..e840e98b 100644 --- a/tests/test_download_manager.py +++ b/tests/test_download_manager.py @@ -249,28 +249,31 @@ class TestDownloadManager(unittest.TestCase): import asyncio async def run_test(): - # Request 404 error from httpbin - data = await DownloadManager.download_url("https://httpbin.org/status/404") + # Request 404 error from httpbin - should raise RuntimeError + with self.assertRaises(RuntimeError) as context: + data = await DownloadManager.download_url("https://httpbin.org/status/404") - # Should return None for memory download - self.assertIsNone(data) + # Should raise RuntimeError with status code + self.assertIn("404", str(context.exception)) asyncio.run(run_test()) def test_http_error_with_file_output(self): - """Test that file download returns False on HTTP error.""" + """Test that file download raises exception on HTTP error.""" import asyncio async def run_test(): outfile = f"{self.temp_dir}/error_test.bin" - success = await DownloadManager.download_url( - "https://httpbin.org/status/500", - outfile=outfile - ) + # Should raise RuntimeError for HTTP 500 + with self.assertRaises(RuntimeError) as context: + success = await DownloadManager.download_url( + "https://httpbin.org/status/500", + outfile=outfile + ) - # Should return False for file download - self.assertFalse(success) + # Should raise RuntimeError with status code + self.assertIn("500", str(context.exception)) # File should not be created try: @@ -286,14 +289,9 @@ class TestDownloadManager(unittest.TestCase): import asyncio async def run_test(): - # Invalid URL should raise exception or return None - try: + # Invalid URL should raise an exception + with self.assertRaises(Exception): data = await DownloadManager.download_url("http://invalid-url-that-does-not-exist.local/") - # If it doesn't raise, it should return None - self.assertIsNone(data) - except Exception: - # Exception is acceptable - pass asyncio.run(run_test()) @@ -372,16 +370,12 @@ class TestDownloadManager(unittest.TestCase): # Try to download to non-existent directory outfile = "/tmp/nonexistent_dir_12345/test.bin" - try: + # Should raise exception because directory doesn't exist + with self.assertRaises(Exception): success = await DownloadManager.download_url( "https://httpbin.org/bytes/100", outfile=outfile ) - # Should fail because directory doesn't exist - self.assertFalse(success) - except Exception: - # Exception is acceptable - pass asyncio.run(run_test())