Hopefully fixed filepath bug in modal deployment

Author: beveradb

audio_separator/remote/api_client.py

@@ -7,6 +7,17 @@
 
 import requests
 
+# Get package version for debugging
+try:
+    from importlib.metadata import version
+    AUDIO_SEPARATOR_VERSION = version("audio-separator")
+except ImportError:
+    try:
+        import pkg_resources
+        AUDIO_SEPARATOR_VERSION = pkg_resources.get_distribution("audio-separator").version
+    except Exception:
+        AUDIO_SEPARATOR_VERSION = "unknown"
+
 
 class AudioSeparatorAPIClient:
     """Client for interacting with a remotely deployed Audio Separator API."""
@@ -198,7 +209,7 @@ def separate_audio_and_wait(
 
         # Submit the separation job with all parameters
         models_desc = models or ([model] if model else ["default"])
-        self.logger.info(f"Submitting separation job for '{file_path}' with models: {models_desc}")
+        self.logger.info(f"Submitting separation job for '{file_path}' with models: {models_desc} (audio-separator v{AUDIO_SEPARATOR_VERSION})")
 
         result = self.separate_audio(
             file_path,
@@ -262,16 +273,27 @@ def separate_audio_and_wait(
                 # Check if completed
                 if current_status == "completed":
                     self.logger.info("✅ Separation completed!")
+                    
+                    files_list = status.get("files", [])
+                    self.logger.info(f"🔍 Job status returned {len(files_list)} files")
+                    for i, file in enumerate(files_list):
+                        self.logger.info(f"  [{i}] '{file}' (len={len(file)}, repr={repr(file)})")
 
-                    result = {"task_id": task_id, "status": "completed", "files": status.get("files", [])}
+                    result = {"task_id": task_id, "status": "completed", "files": files_list}
 
                     # Download files if requested
                     if download:
                         downloaded_files = []
-                        self.logger.info(f"📥 Downloading {len(status.get('files', []))} output files...")
+                        files_list = status.get("files", [])
+                        self.logger.info(f"📥 Downloading {len(files_list)} output files...")
+                        
+                        # Log the filenames we're trying to download for debugging
+                        self.logger.info(f"🔍 Files to download: {files_list}")
 
-                        for filename in status.get("files", []):
+                        for i, filename in enumerate(files_list):
                             try:
+                                self.logger.info(f"🔍 [{i+1}/{len(files_list)}] Attempting to download: '{filename}' (len={len(filename)})")
+                                
                                 if output_dir:
                                     output_path = f"{output_dir.rstrip('/')}/{filename}"
                                 else:
@@ -282,6 +304,13 @@ def separate_audio_and_wait(
                                 self.logger.info(f"  ✅ Downloaded: {downloaded_path}")
                             except Exception as e:
                                 self.logger.error(f"  ❌ Failed to download {filename}: {e}")
+                                
+                                # Try to get server version on download failure for debugging
+                                try:
+                                    server_version = self.get_server_version()
+                                    self.logger.error(f"🔍 Server version when download failed: {server_version}")
+                                except Exception as version_error:
+                                    self.logger.error(f"🔍 Could not get server version: {version_error}")
 
                         result["downloaded_files"] = downloaded_files
                         self.logger.info(f"🎉 Successfully downloaded {len(downloaded_files)} files!")
@@ -322,7 +351,29 @@ def download_file(self, task_id: str, filename: str, output_path: Optional[str]
         try:
             # URL encode the filename to handle spaces and special characters
             encoded_filename = quote(filename, safe='')
-            response = self.session.get(f"{self.api_url}/download/{task_id}/{encoded_filename}", timeout=60)
+            download_url = f"{self.api_url}/download/{task_id}/{encoded_filename}"
+            
+            # Debug logging to understand what's happening
+            self.logger.info(f"🔍 Download details:")
+            self.logger.info(f"  Original filename: '{filename}'")
+            self.logger.info(f"  Encoded filename: '{encoded_filename}'")
+            self.logger.info(f"  Download URL: {download_url}")
+            self.logger.info(f"  Task ID: {task_id}")
+            
+            response = self.session.get(download_url, timeout=60)
+            
+            # Log response details for debugging
+            self.logger.info(f"🔍 Response status: {response.status_code}")
+            if response.status_code != 200:
+                try:
+                    self.logger.error(f"🔍 Response headers: {dict(response.headers)}")
+                except Exception:
+                    self.logger.error(f"🔍 Response headers: {response.headers}")
+                try:
+                    self.logger.error(f"🔍 Response text (first 500 chars): {response.text[:500]}")
+                except Exception:
+                    self.logger.error(f"🔍 Response text: <unavailable>")
+            
             response.raise_for_status()
 
             with open(output_path, "wb") as f:

audio_separator/remote/deploy_modal.py

@@ -30,6 +30,7 @@
 from importlib.metadata import version
 import typing
 from typing import Optional
+from urllib.parse import unquote
 
 # Third-party imports
 from fastapi import FastAPI, File, Form, HTTPException, Response, UploadFile
@@ -102,7 +103,7 @@
     .pip_install(
         [
             # Core audio-separator with GPU support (this pulls in most dependencies from pyproject.toml)
-            "audio-separator[gpu]>=0.35.0",
+            "audio-separator[gpu]>=0.35.1",
             # FastAPI and web server dependencies for Modal API deployment
             "fastapi>=0.104.0",
             "uvicorn[standard]>=0.24.0",
@@ -616,7 +617,12 @@ async def download_file(task_id: str, filename: str) -> Response:
     Download a separated audio file
     """
     try:
-        file_data = get_file_function.remote(task_id, filename)
+        # Explicitly URL-decode the filename to handle edge cases with special characters
+        # FastAPI normally handles this, but complex filenames with multiple special chars
+        # may not be decoded properly in all cases
+        decoded_filename = unquote(filename)
+        
+        file_data = get_file_function.remote(task_id, decoded_filename)
 
         # Detect file type from content
         detected_type = filetype.guess(file_data)
@@ -625,10 +631,10 @@ async def download_file(task_id: str, filename: str) -> Response:
             content_type = detected_type.mime
         else:
             # Log when we can't detect the file type
-            print(f"WARNING: Could not detect MIME type for {filename}, using generic type")
+            print(f"WARNING: Could not detect MIME type for {decoded_filename}, using generic type")
             content_type = "application/octet-stream"
 
-        return Response(content=file_data, media_type=content_type, headers={"Content-Disposition": f"attachment; filename={filename}"})
+        return Response(content=file_data, media_type=content_type, headers={"Content-Disposition": f"attachment; filename={decoded_filename}"})
 
     except FileNotFoundError as exc:
         raise HTTPException(status_code=404, detail="File not found") from exc

poetry.lock

@@ -73,6 +73,7 @@ pytest-cov = ">=4.1.0"
 matplotlib = ">=3.8.0"
 pillow = ">=10.1.0"
 scikit-image = ">=0.22.0"
+filetype = ">=1"
 
 [tool.black]
 line-length = 140

pyproject.toml

@@ -161,6 +161,7 @@ def test_download_file_success(self, mock_file, mock_get, api_client):
         """Test successful file download."""
         mock_response = Mock()
         mock_response.content = b"fake audio file content"
+        mock_response.status_code = 200
         mock_response.raise_for_status.return_value = None
         mock_get.return_value = mock_response
 
@@ -176,6 +177,7 @@ def test_download_file_default_output_path(self, mock_file, mock_get, api_client
         """Test file download with default output path."""
         mock_response = Mock()
         mock_response.content = b"fake audio file content"
+        mock_response.status_code = 200
         mock_response.raise_for_status.return_value = None
         mock_get.return_value = mock_response
 
@@ -190,6 +192,7 @@ def test_download_file_with_spaces_in_filename(self, mock_file, mock_get, api_cl
         """Test file download with spaces in filename (URL encoding)."""
         mock_response = Mock()
         mock_response.content = b"fake audio file content"
+        mock_response.status_code = 200
         mock_response.raise_for_status.return_value = None
         mock_get.return_value = mock_response
 
@@ -208,6 +211,7 @@ def test_download_file_with_special_characters(self, mock_file, mock_get, api_cl
         """Test file download with special characters in filename."""
         mock_response = Mock()
         mock_response.content = b"fake audio file content"
+        mock_response.status_code = 200
         mock_response.raise_for_status.return_value = None
         mock_get.return_value = mock_response
 
@@ -226,6 +230,7 @@ def test_download_file_with_unicode_characters(self, mock_file, mock_get, api_cl
         """Test file download with unicode characters in filename."""
         mock_response = Mock()
         mock_response.content = b"fake audio file content"
+        mock_response.status_code = 200
         mock_response.raise_for_status.return_value = None
         mock_get.return_value = mock_response
 
@@ -438,3 +443,23 @@ def test_separate_audio_and_wait_with_special_character_filenames(self, mock_sle
         ]
         actual_calls = [call.args for call in mock_download.call_args_list]
         assert actual_calls == expected_calls
+
+    @patch("requests.Session.get")
+    @patch("builtins.open", new_callable=mock_open)
+    def test_download_file_server_side_url_decoding_scenario(self, mock_file, mock_get, api_client):
+        """Test that the client properly URL-encodes filenames that require server-side decoding."""
+        mock_response = Mock()
+        mock_response.content = b"fake audio file content"
+        mock_response.status_code = 200
+        mock_response.raise_for_status.return_value = None
+        mock_get.return_value = mock_response
+
+        # Test the exact problematic filename from the bug report
+        problematic_filename = "Bloc Party - The Prayer (Vocals model_bs_roformer_ep_317_sdr_12.9755.ckpt)_(Instrumental)_mel_band_roformer_karaoke_aufr33_viperx_sdr_10.flac"
+        result = api_client.download_file("test-task-bug", problematic_filename)
+
+        # Verify URL was properly encoded - this is the exact URL that should be sent
+        expected_url = "https://test-api.example.com/download/test-task-bug/Bloc%20Party%20-%20The%20Prayer%20%28Vocals%20model_bs_roformer_ep_317_sdr_12.9755.ckpt%29_%28Instrumental%29_mel_band_roformer_karaoke_aufr33_viperx_sdr_10.flac"
+        mock_get.assert_called_once_with(expected_url, timeout=60)
+        assert result == problematic_filename
+        mock_file.assert_called_once_with(problematic_filename, "wb")