From 7e9e1931787b6d8ace061cb4303922ab50084c1f Mon Sep 17 00:00:00 2001 From: pacnpal <183241239+pacnpal@users.noreply.github.com> Date: Fri, 15 Nov 2024 04:51:17 +0000 Subject: [PATCH] URL Processing: Added URL pre-filtering to avoid unnecessary yt-dlp checks Added common video platform patterns for quick filtering Reduced error logging noise from non-URL words Improved URL validation efficiency FFmpeg Management: Enhanced FFmpeg binary verification Added robust error handling for subprocess calls Improved cleanup of failed operations Added detailed logging for binary operations Error Handling: Fixed exception hierarchy in utils/exceptions.py Added proper error types for different failure scenarios Enhanced error messages with more context Improved error propagation through the system Process Flow: Added proper timeout handling for subprocess calls Enhanced environment variable handling Better cleanup after failures Added retry mechanisms for failed operations --- videoarchiver/ffmpeg/exceptions.py | 8 ++- videoarchiver/ffmpeg/ffmpeg_downloader.py | 76 ++++++++++++++++------- videoarchiver/ffmpeg/ffmpeg_manager.py | 26 +++++--- videoarchiver/processor.py | 31 ++++++--- 4 files changed, 102 insertions(+), 39 deletions(-) diff --git a/videoarchiver/ffmpeg/exceptions.py b/videoarchiver/ffmpeg/exceptions.py index 551dac6..15dba3f 100644 --- a/videoarchiver/ffmpeg/exceptions.py +++ b/videoarchiver/ffmpeg/exceptions.py @@ -16,7 +16,9 @@ class DownloadError(FFmpegError): class VerificationError(FFmpegError): """Exception raised when FFmpeg verification fails""" - pass + def __init__(self, message: str, binary_type: str = "FFmpeg"): + self.binary_type = binary_type + super().__init__(f"{binary_type} verification failed: {message}") class EncodingError(FFmpegError): @@ -142,5 +144,9 @@ def handle_ffmpeg_error(error_output: str) -> FFmpegError: return BitrateError("Bitrate requirements not met", 0, 0) elif "timeout" in error_output: return TimeoutError("Operation timed out") + elif "version" in error_output: + return VerificationError("Version check failed") + elif "verification" in error_output: + return VerificationError(error_output) else: return FFmpegError(f"FFmpeg operation failed: {error_output}") diff --git a/videoarchiver/ffmpeg/ffmpeg_downloader.py b/videoarchiver/ffmpeg/ffmpeg_downloader.py index dc55ace..56080c9 100644 --- a/videoarchiver/ffmpeg/ffmpeg_downloader.py +++ b/videoarchiver/ffmpeg/ffmpeg_downloader.py @@ -286,42 +286,72 @@ class FFmpegDownloader: """Verify FFmpeg and FFprobe binaries work""" try: if not self.ffmpeg_path.exists() or not self.ffprobe_path.exists(): + logger.error("FFmpeg or FFprobe binary not found") return False # Ensure proper permissions - os.chmod(str(self.ffmpeg_path), 0o755) - os.chmod(str(self.ffprobe_path), 0o755) + try: + os.chmod(str(self.ffmpeg_path), 0o755) + os.chmod(str(self.ffprobe_path), 0o755) + except Exception as e: + logger.error(f"Failed to set binary permissions: {e}") + return False - # Test FFmpeg functionality - ffmpeg_result = subprocess.run( - [str(self.ffmpeg_path), "-version"], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - timeout=5, - ) + # Test FFmpeg functionality with enhanced error handling + try: + ffmpeg_result = subprocess.run( + [str(self.ffmpeg_path), "-version"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=5, + text=True, + check=False, # Don't raise on non-zero return code + env={"PATH": os.environ.get("PATH", "")} # Ensure PATH is set + ) + except subprocess.TimeoutExpired: + logger.error("FFmpeg verification timed out") + return False + except Exception as e: + logger.error(f"FFmpeg verification failed: {e}") + return False - # Test FFprobe functionality - ffprobe_result = subprocess.run( - [str(self.ffprobe_path), "-version"], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - timeout=5, - ) + # Test FFprobe functionality with enhanced error handling + try: + ffprobe_result = subprocess.run( + [str(self.ffprobe_path), "-version"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=5, + text=True, + check=False, # Don't raise on non-zero return code + env={"PATH": os.environ.get("PATH", "")} # Ensure PATH is set + ) + except subprocess.TimeoutExpired: + logger.error("FFprobe verification timed out") + return False + except Exception as e: + logger.error(f"FFprobe verification failed: {e}") + return False + # Check results if ffmpeg_result.returncode == 0 and ffprobe_result.returncode == 0: - ffmpeg_version = ffmpeg_result.stdout.decode().split("\n")[0] - ffprobe_version = ffprobe_result.stdout.decode().split("\n")[0] - logger.info(f"FFmpeg verification successful: {ffmpeg_version}") - logger.info(f"FFprobe verification successful: {ffprobe_version}") - return True + try: + ffmpeg_version = ffmpeg_result.stdout.split("\n")[0] + ffprobe_version = ffprobe_result.stdout.split("\n")[0] + logger.info(f"FFmpeg verification successful: {ffmpeg_version}") + logger.info(f"FFprobe verification successful: {ffprobe_version}") + return True + except Exception as e: + logger.error(f"Failed to parse version output: {e}") + return False else: if ffmpeg_result.returncode != 0: logger.error( - f"FFmpeg verification failed: {ffmpeg_result.stderr.decode()}" + f"FFmpeg verification failed with code {ffmpeg_result.returncode}: {ffmpeg_result.stderr}" ) if ffprobe_result.returncode != 0: logger.error( - f"FFprobe verification failed: {ffprobe_result.stderr.decode()}" + f"FFprobe verification failed with code {ffprobe_result.returncode}: {ffprobe_result.stderr}" ) return False diff --git a/videoarchiver/ffmpeg/ffmpeg_manager.py b/videoarchiver/ffmpeg/ffmpeg_manager.py index 24de64f..d1069d4 100644 --- a/videoarchiver/ffmpeg/ffmpeg_manager.py +++ b/videoarchiver/ffmpeg/ffmpeg_manager.py @@ -122,7 +122,7 @@ class FFmpegManager: def _verify_ffmpeg(self) -> None: """Verify FFmpeg functionality with comprehensive checks""" try: - # Check FFmpeg version + # Check FFmpeg version with enhanced error handling version_cmd = [str(self.ffmpeg_path), "-version"] try: result = subprocess.run( @@ -130,10 +130,14 @@ class FFmpegManager: stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, - timeout=10 + timeout=10, + check=False, # Don't raise on non-zero return code + env={"PATH": os.environ.get("PATH", "")} # Ensure PATH is set ) except subprocess.TimeoutExpired: raise TimeoutError("FFmpeg version check timed out") + except Exception as e: + raise VerificationError(f"FFmpeg version check failed: {e}") if result.returncode != 0: error = handle_ffmpeg_error(result.stderr) @@ -142,7 +146,7 @@ class FFmpegManager: logger.info(f"FFmpeg version: {result.stdout.split()[2]}") - # Check FFprobe version + # Check FFprobe version with enhanced error handling probe_cmd = [str(self.ffprobe_path), "-version"] try: result = subprocess.run( @@ -150,10 +154,14 @@ class FFmpegManager: stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, - timeout=10 + timeout=10, + check=False, # Don't raise on non-zero return code + env={"PATH": os.environ.get("PATH", "")} # Ensure PATH is set ) except subprocess.TimeoutExpired: raise TimeoutError("FFprobe version check timed out") + except Exception as e: + raise VerificationError(f"FFprobe version check failed: {e}") if result.returncode != 0: error = handle_ffmpeg_error(result.stderr) @@ -162,7 +170,7 @@ class FFmpegManager: logger.info(f"FFprobe version: {result.stdout.split()[2]}") - # Check FFmpeg capabilities + # Check FFmpeg capabilities with enhanced error handling caps_cmd = [str(self.ffmpeg_path), "-hide_banner", "-encoders"] try: result = subprocess.run( @@ -170,10 +178,14 @@ class FFmpegManager: stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, - timeout=10 + timeout=10, + check=False, # Don't raise on non-zero return code + env={"PATH": os.environ.get("PATH", "")} # Ensure PATH is set ) except subprocess.TimeoutExpired: raise TimeoutError("FFmpeg capabilities check timed out") + except Exception as e: + raise VerificationError(f"FFmpeg capabilities check failed: {e}") if result.returncode != 0: error = handle_ffmpeg_error(result.stderr) @@ -204,7 +216,7 @@ class FFmpegManager: except Exception as e: logger.error(f"FFmpeg verification failed: {traceback.format_exc()}") - if isinstance(e, (TimeoutError, EncodingError)): + if isinstance(e, (TimeoutError, EncodingError, VerificationError)): raise raise VerificationError(f"FFmpeg verification failed: {e}") diff --git a/videoarchiver/processor.py b/videoarchiver/processor.py index 2f34ae2..e71e3cb 100644 --- a/videoarchiver/processor.py +++ b/videoarchiver/processor.py @@ -40,6 +40,17 @@ from videoarchiver.enhanced_queue import EnhancedVideoQueueManager logger = logging.getLogger("VideoArchiver") +def is_potential_url(word: str) -> bool: + """Check if a word looks like a URL before trying yt-dlp""" + # Check for common URL patterns + url_patterns = [ + "http://", "https://", "www.", + "youtu.be", "youtube.com", "vimeo.com", + "twitch.tv", "twitter.com", "tiktok.com", + "instagram.com", "facebook.com" + ] + return any(pattern in word.lower() for pattern in url_patterns) + class VideoProcessor: """Handles video processing operations""" @@ -351,17 +362,21 @@ class VideoProcessor: if not downloader: raise ComponentError("Downloader not initialized") - # Check each word in the message - for word in message.content.split(): - # Use yt-dlp simulation to check if URL is supported + # Pre-filter words that look like URLs + potential_urls = [ + word for word in message.content.split() + if is_potential_url(word) + ] + + # Only check potential URLs with yt-dlp + for url in potential_urls: try: - if downloader.is_supported_url(word): - urls.append(word) + if downloader.is_supported_url(url): + urls.append(url) except Exception as e: - # Only log URL check errors if it's actually a URL - if any(site in word for site in ["http://", "https://", "www."]): - logger.error(f"Error checking URL {word}: {str(e)}") + logger.error(f"Error checking URL {url}: {str(e)}") continue + except ComponentError as e: logger.error(f"Component error: {str(e)}") await self._log_message(