From 1f37a67c53233dc8e814d22620e1ff8a6fdb14b6 Mon Sep 17 00:00:00 2001 From: salmonik Date: Wed, 20 May 2026 15:21:37 +0200 Subject: health checks for vc Signed-off-by: salmonik --- cogs/voice.py | 122 ++++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 89 insertions(+), 33 deletions(-) (limited to 'cogs/voice.py') diff --git a/cogs/voice.py b/cogs/voice.py index 046c2a8..6f9a269 100644 --- a/cogs/voice.py +++ b/cogs/voice.py @@ -24,60 +24,116 @@ def get_voice_manager(bot) -> "VoiceManager": return _voice_managers[bot_id] +def is_voice_client_alive(voice_client, guild): + """Check if a voice client connection is healthy. + + Verifies the client is connected, the channel still exists, + and the underlying voice socket is still open. + """ + if voice_client is None: + return False + if not voice_client.is_connected(): + return False + if voice_client.channel is None: + return False + if guild.get_channel(voice_client.channel.id) is None: + return False + + try: + ws = voice_client._connection + s = ws.socket + if s is None or s.fileno() == -1: + return False + except Exception: + return False + + return True + + @dataclass class VoiceManager: bot: Any # type: ignore[no-any-import] _voice_lock: asyncio.Lock = field(default_factory=asyncio.Lock, init=False) + _connecting: set[int] = field(default_factory=set, init=False) async def join_channel(self, channel): """Join a voice channel by channel object.""" async with self._voice_lock: guild = channel.guild + guild_id = guild.id + + if guild_id in self._connecting: + logger.info(f"Connection already in progress for guild {guild_id}, skipping") + return None + voice_client = discord.utils.get(self.bot.voice_clients, guild=guild) if voice_client and voice_client.is_connected(): if voice_client.channel == channel: - logger.info(f"Already connected to {channel}") - return voice_client - try: - await voice_client.move_to(channel) - logger.info(f"Moved to voice channel: {channel}") - return voice_client - except Exception as e: - logger.error(f"Failed to move voice client: {e}") - logger.info("Disconnecting after failed move attempt") + if is_voice_client_alive(voice_client, guild): + logger.info(f"Already connected to {channel}") + return voice_client + logger.warning(f"Stale connection to {channel}, reconnecting") try: await voice_client.disconnect() except Exception: pass - - await asyncio.sleep(0.3) - try: - voice_client = await channel.connect(timeout=30.0, reconnect=False) - logger.info(f"Connected to voice channel: {channel}") - return voice_client - except discord.ClientException as e: - if "Already connected" in str(e): - logger.warning(f"Stale voice state detected, forcing disconnect and reconnect: {e}") - voice_client = discord.utils.get(self.bot.voice_clients, guild=guild) - if voice_client: + else: + try: + await voice_client.move_to(channel) + logger.info(f"Moved to voice channel: {channel}") + return voice_client + except Exception as e: + logger.error(f"Failed to move voice client: {e}") + logger.info("Disconnecting after failed move attempt") try: await voice_client.disconnect() except Exception: pass - await asyncio.sleep(0.3) - try: - voice_client = await channel.connect(timeout=30.0, reconnect=False) - logger.info(f"Reconnected to voice channel: {channel}") - return voice_client - except Exception as e2: - logger.error(f"Failed to reconnect after stale cleanup: {e2}") - return None - logger.error(f"Failed to connect to voice channel: {e}") - return None - except Exception as e: - logger.error(f"Failed to connect to voice channel: {e}") - return None + elif voice_client and not voice_client.is_connected(): + logger.info("Voice client exists but disconnected, cleaning up stale entry") + try: + await voice_client.disconnect() + except Exception: + pass + voice_client = None + + self._connecting.add(guild_id) + try: + await asyncio.sleep(0.3) + try: + voice_client = await channel.connect(timeout=30.0, reconnect=False) + logger.info(f"Connected to voice channel: {channel}") + return voice_client + except discord.ClientException as e: + if "Already connected" in str(e): + logger.warning(f"Stale voice state detected, forcing disconnect and reconnect: {e}") + voice_client = discord.utils.get(self.bot.voice_clients, guild=guild) + if voice_client: + try: + await voice_client.disconnect() + except Exception: + pass + else: + try: + channel._state._remove_voice_client(guild.id) + except Exception: + pass + await asyncio.sleep(0.3) + try: + voice_client = await channel.connect(timeout=30.0, reconnect=False) + logger.info(f"Reconnected to voice channel: {channel}") + return voice_client + except Exception as e2: + logger.error(f"Failed to reconnect after stale cleanup: {e2}") + return None + logger.error(f"Failed to connect to voice channel: {e}") + return None + except Exception as e: + logger.error(f"Failed to connect to voice channel: {e}") + return None + finally: + self._connecting.discard(guild_id) async def join_by_channel_id(self, channel_id): """Join a voice channel by channel ID.""" -- cgit v1.2.3 From fbdfad855ab9a1233e286806439397729ba21226 Mon Sep 17 00:00:00 2001 From: salmonik Date: Wed, 20 May 2026 15:36:17 +0200 Subject: Fold AudioPlaybackManager into VoiceManager + Future-based play - Move all static helpers (validate_voice_channel, prepare_source, prepare_audio_source, etc.) from AudioPlaybackManager into VoiceManager - Add _play_await() using asyncio.Future + after callback, replacing all polling patterns and the 5-second sleep hack - Add play() high-level method and play_source() for pre-made sources - Unify connection logic under single _ensure_voice_client() - Remove dead Voice cog class and its setup() - Gut audio_playback.py to a re-export shim - Remove cogs.voice from main.py extensions --- cogs/music/audio_playback.py | 292 +------------------------------ cogs/voice.py | 407 +++++++++++++++++++++++++++---------------- main.py | 1 - 3 files changed, 258 insertions(+), 442 deletions(-) (limited to 'cogs/voice.py') diff --git a/cogs/music/audio_playback.py b/cogs/music/audio_playback.py index 0c885c3..5206a96 100644 --- a/cogs/music/audio_playback.py +++ b/cogs/music/audio_playback.py @@ -3,292 +3,8 @@ # Copyright (C) 2026 Salmonik @ salmonik@tuta.io # https://codeberg.org/salmonik/senator-sperma -import asyncio -import logging -import os -import urllib.parse +# Re-export shim — all voice utilities now live in cogs.voice.VoiceManager. +# Kept so existing imports in play_commands.py and ai.py keep working. +# TODO: update importers and remove this file. -import discord -import mutagen - -logger = logging.getLogger(__name__) - - -class AudioPlaybackManager: - """Handles audio playback, voice connections, and source preparation.""" - - @staticmethod - async def cleanup_temp_file(source): - """Clean up temporary file if it exists.""" - if source and os.path.exists(source): - try: - os.remove(source) - logger.info(f"Cleaned up temporary file: {source}") - except Exception as e: - logger.error(f"Error cleaning up file {source}: {str(e)}") - - @staticmethod - async def validate_voice_channel(ctx): - """Validate user is in a voice channel. Returns channel or None.""" - if not ctx.author.voice or not ctx.author.voice.channel: - await ctx.respond( - "Musisz być na kanale głosowym, żebym mógł dołączyć!", ephemeral=True - ) - return None - return ctx.author.voice.channel - - @staticmethod - async def prepare_file_source(source): - """Prepare and validate local file source. Returns (display_name, absolute_path) or (None, None).""" - logger.info(f"Attempting to play local file: {source}") - - # Get absolute path and verify file exists - if not os.path.isabs(source): - source = os.path.abspath(source) - logger.debug(f"Converted to absolute path: {source}") - - if not os.path.exists(source): - logger.error(f"File not found: {source}") - return None, None - - filename = os.path.basename(source) - display_name = os.path.splitext(filename)[0] - return display_name, source - - @staticmethod - async def connect_to_voice_channel(ctx, vc, target_channel): - """Connect or move to voice channel. Returns voice client.""" - try: - # Check if vc is valid and actually connected - if vc is not None and vc.is_connected(): - # Already connected - check if need to move - if vc.channel != target_channel: - logger.info(f"Moving to voice channel: {target_channel.name}") - await vc.move_to(target_channel) - logger.info(f"Moved to voice channel: {target_channel.name}") - return vc - - # Not connected - connect now - logger.info(f"Connecting to voice channel: {target_channel.name}") - vc = await target_channel.connect() - logger.info(f"Connected to voice channel: {target_channel.name}") - return vc - except Exception as e: - logger.error(f"Error connecting to voice channel: {str(e)}") - raise - - @staticmethod - async def prepare_audio_source(source, is_url, display_name): - """Prepare audio source for playback. Returns (source, final_display_name).""" - if is_url: - # For URLs, use FFmpeg with the direct URL and extract clean filename - before_options = "-reconnect 1 -reconnect_streamed 1 -reconnect_delay_max 5" - audio_source = discord.FFmpegPCMAudio(source, before_options=before_options) - # Extract clean filename for URL (remove extension and decode URL encoding) - filename = os.path.basename(source.split("?")[0]) # Remove query parameters - filename = urllib.parse.unquote(filename) # Decode URL encoding - clean_filename = os.path.splitext(filename)[0] # Remove file extension - return audio_source, clean_filename - else: - # For local files, use default options - audio_source = discord.FFmpegPCMAudio(source=source) - - # Try to get metadata for local files - try: - metadata = mutagen.open(source) # type: ignore[call-overload] - if metadata: - title = ( - metadata.get("title", [None])[0] - if hasattr(metadata, "get") - else None - ) - if title: - display_name = title - logger.debug(f"Read metadata - title: {title}") - else: - display_name = os.path.splitext(os.path.basename(source))[0] - except Exception as e: - logger.warning(f"Could not read metadata from {source}: {str(e)}") - display_name = os.path.splitext(os.path.basename(source))[0] - - return audio_source, display_name - - @staticmethod - async def cleanup_if_needed(source, delete_after_play, is_url): - """Clean up temporary file if needed.""" - if delete_after_play and not is_url: - await AudioPlaybackManager.cleanup_temp_file(source) - - @staticmethod - async def prepare_source(ctx, source): - """Prepare source and get display name. Returns (source, display_name, is_url) or (None, None, None).""" - is_url = source.startswith(("http://", "https://")) - - if is_url: - logger.info(f"Attempting to play stream from URL: {source}") - return source, source, is_url - - display_name, absolute_source = await AudioPlaybackManager.prepare_file_source( - source - ) - if not display_name: - return None, None, None - - return absolute_source, display_name, is_url - - @staticmethod - async def execute_playback(ctx, source, display_name, is_url, interrupt=True): - """Execute the actual audio playback. - - Args: - ctx: Discord context - source: Audio source path or URL - display_name: Name to display - is_url: Whether source is a URL - interrupt: If True, stop any currently playing audio. If False, skip if audio is playing. - """ - target_channel = await AudioPlaybackManager.validate_voice_channel(ctx) - if not target_channel: - return False - - # Get existing voice client from bot or connect - guild = ctx.guild - vc = discord.utils.get(ctx.bot.voice_clients, guild=guild) - - try: - # Handle various connection states - if vc is None: - logger.info("No existing voice client, connecting...") - try: - vc = await target_channel.connect() - except discord.ClientException: - vc = discord.utils.get(ctx.bot.voice_clients, guild=guild) - elif not vc.is_connected(): - logger.info("Voice client exists but disconnected, reconnecting...") - try: - await vc.disconnect() - except Exception: - pass - try: - vc = await target_channel.connect() - except discord.ClientException: - vc = discord.utils.get(ctx.bot.voice_clients, guild=guild) - elif vc.channel == target_channel: - from cogs.voice import is_voice_client_alive - if not is_voice_client_alive(vc, guild): - logger.warning("Stale voice client, reconnecting...") - try: - await vc.disconnect() - except Exception: - pass - try: - vc = await target_channel.connect() - except discord.ClientException: - vc = discord.utils.get(ctx.bot.voice_clients, guild=guild) - elif vc.channel != target_channel: - logger.info(f"Moving to voice channel: {target_channel.name}") - await vc.move_to(target_channel) - - # Stop any currently playing audio (if interrupt is True) - if interrupt and (vc.is_playing() or vc.is_paused()): # type: ignore[union-attr] - vc.stop() # type: ignore[union-attr] - elif not interrupt and vc.is_playing(): # type: ignore[union-attr] - logger.info("Audio already playing and interrupt=False, skipping playback") - return False - - # Prepare audio source - ( - audio_source, - final_display_name, - ) = await AudioPlaybackManager.prepare_audio_source( - source, is_url, display_name - ) - - # Track playback state - playback_state = {"error": None, "finished": False} - - def after_playing(error): - if error: - playback_state["error"] = str(error) - logger.error(f"Playback error: {error}") - playback_state["finished"] = True - - # Play the audio - vc.play(audio_source, after=after_playing) - logger.info(f"Playing: {final_display_name}") - await ctx.followup.send(f"grom: {final_display_name}") - - # Wait for FFmpeg to attempt connection - errors usually happen within 5 seconds - await asyncio.sleep(5) - - # Check if playback failed to start (not just finished quickly) - if playback_state["error"]: - logger.error(f"Playback failed: {playback_state['error']}") - await ctx.followup.send( - "eror: Timeout - nie udało się połączyć ze streamem", ephemeral=True - ) - return False - - # If playback finished within 5s, it was a short file - not an error - if playback_state["finished"]: - logger.info(f"Playback completed quickly: {final_display_name}") - return True - - if not vc.is_playing(): - logger.error(f"Stream failed - not playing after 5s: {source}") - await ctx.followup.send( - "eror: Timeout - nie udało się połączyć ze streamem", ephemeral=True - ) - return False - - return True - - except Exception as e: - logger.error(f"Error in play: {str(e)}", exc_info=True) - await ctx.followup.send(f"eror: {str(e)}", ephemeral=True) - return False - - @staticmethod - async def validate_and_prepare_source(ctx, source): - """Validate and prepare source for playback. Returns (source, display_name, is_url) or (None, None, None).""" - ( - prepared_source, - display_name, - is_url, - ) = await AudioPlaybackManager.prepare_source(ctx, source) - if not prepared_source: - return None, None, None - return prepared_source, display_name, is_url - - @staticmethod - async def play(ctx, source, delete_after_play=False, interrupt=True): - """Play audio from a local file or URL. - - Args: - ctx: Discord context - source: Either a local file path or a URL to stream from - delete_after_play: If True, delete the file after playing (only for local files) - interrupt: If True, stop any currently playing audio. If False, skip if audio is playing. - """ - # Validate and prepare source - ( - prepared_source, - display_name, - is_url, - ) = await AudioPlaybackManager.validate_and_prepare_source(ctx, source) - if not prepared_source: - await AudioPlaybackManager.cleanup_if_needed( - source, delete_after_play, False - ) - return - - # Execute playback - success = await AudioPlaybackManager.execute_playback( - ctx, prepared_source, display_name, is_url, interrupt - ) - - # Cleanup if needed - if not success: - await AudioPlaybackManager.cleanup_if_needed( - prepared_source, delete_after_play, is_url - ) +from cogs.voice import VoiceManager as AudioPlaybackManager # noqa: F401 diff --git a/cogs/voice.py b/cogs/voice.py index 6f9a269..ba1993e 100644 --- a/cogs/voice.py +++ b/cogs/voice.py @@ -4,12 +4,15 @@ # https://codeberg.org/salmonik/senator-sperma import discord -from discord.ext import commands import logging import asyncio +import os +import urllib.parse from dataclasses import dataclass, field from typing import Any +import mutagen + logger = logging.getLogger(__name__) @@ -25,11 +28,7 @@ def get_voice_manager(bot) -> "VoiceManager": def is_voice_client_alive(voice_client, guild): - """Check if a voice client connection is healthy. - - Verifies the client is connected, the channel still exists, - and the underlying voice socket is still open. - """ + """Check if a voice client connection is healthy.""" if voice_client is None: return False if not voice_client.is_connected(): @@ -52,66 +51,75 @@ def is_voice_client_alive(voice_client, guild): @dataclass class VoiceManager: - bot: Any # type: ignore[no-any-import] - _voice_lock: asyncio.Lock = field(default_factory=asyncio.Lock, init=False) + bot: Any + _lock: asyncio.Lock = field(default_factory=asyncio.Lock, init=False) _connecting: set[int] = field(default_factory=set, init=False) - async def join_channel(self, channel): - """Join a voice channel by channel object.""" - async with self._voice_lock: - guild = channel.guild - guild_id = guild.id + # --- Connection ----------------------------------------------------------- + + async def _ensure_voice_client(self, channel): + """Unified connect/move/reconnect logic. + + Returns a healthy VoiceClient connected to *channel*, or None on failure. + """ + guild = channel.guild + guild_id = guild.id + async with self._lock: if guild_id in self._connecting: logger.info(f"Connection already in progress for guild {guild_id}, skipping") return None - voice_client = discord.utils.get(self.bot.voice_clients, guild=guild) + vc = discord.utils.get(self.bot.voice_clients, guild=guild) - if voice_client and voice_client.is_connected(): - if voice_client.channel == channel: - if is_voice_client_alive(voice_client, guild): - logger.info(f"Already connected to {channel}") - return voice_client - logger.warning(f"Stale connection to {channel}, reconnecting") - try: - await voice_client.disconnect() - except Exception: - pass - else: - try: - await voice_client.move_to(channel) - logger.info(f"Moved to voice channel: {channel}") - return voice_client - except Exception as e: - logger.error(f"Failed to move voice client: {e}") - logger.info("Disconnecting after failed move attempt") + # --- resolve existing client -------------------------------------- + if vc is not None: + if vc.is_connected(): + if vc.channel == channel: + if is_voice_client_alive(vc, guild): + logger.info(f"Already connected to {channel}") + return vc + logger.warning(f"Stale connection to {channel}, reconnecting") try: - await voice_client.disconnect() + await vc.disconnect() except Exception: pass - elif voice_client and not voice_client.is_connected(): - logger.info("Voice client exists but disconnected, cleaning up stale entry") - try: - await voice_client.disconnect() - except Exception: - pass - voice_client = None + vc = None + else: + try: + await vc.move_to(channel) + logger.info(f"Moved to voice channel: {channel}") + return vc + except Exception as e: + logger.error(f"Failed to move voice client: {e}") + try: + await vc.disconnect() + except Exception: + pass + vc = None + else: + logger.info("Voice client exists but disconnected, cleaning up stale entry") + try: + await vc.disconnect() + except Exception: + pass + vc = None + # --- create fresh connection -------------------------------------- self._connecting.add(guild_id) try: await asyncio.sleep(0.3) try: - voice_client = await channel.connect(timeout=30.0, reconnect=False) + vc = await channel.connect(timeout=30.0, reconnect=False) logger.info(f"Connected to voice channel: {channel}") - return voice_client + return vc except discord.ClientException as e: if "Already connected" in str(e): - logger.warning(f"Stale voice state detected, forcing disconnect and reconnect: {e}") - voice_client = discord.utils.get(self.bot.voice_clients, guild=guild) - if voice_client: + logger.warning(f"Stale voice state detected, forcing reconnect: {e}") + vc = discord.utils.get(self.bot.voice_clients, guild=guild) + if vc: try: - await voice_client.disconnect() + await vc.disconnect() except Exception: pass else: @@ -121,9 +129,9 @@ class VoiceManager: pass await asyncio.sleep(0.3) try: - voice_client = await channel.connect(timeout=30.0, reconnect=False) + vc = await channel.connect(timeout=30.0, reconnect=False) logger.info(f"Reconnected to voice channel: {channel}") - return voice_client + return vc except Exception as e2: logger.error(f"Failed to reconnect after stale cleanup: {e2}") return None @@ -135,144 +143,237 @@ class VoiceManager: finally: self._connecting.discard(guild_id) - async def join_by_channel_id(self, channel_id): - """Join a voice channel by channel ID.""" - channel = self.bot.get_channel(channel_id) - if not channel: - logger.error(f"Channel {channel_id} not found") - return None - if not isinstance(channel, discord.VoiceChannel): - logger.error(f"Channel {channel_id} is not a voice channel") - return None - return await self.join_channel(channel) + async def connect(self, channel): + """Join a voice channel (public alias).""" + return await self._ensure_voice_client(channel) - async def get_voice_client(self, guild): + async def join_channel(self, channel): + """Backward-compatible alias for connect().""" + return await self.connect(channel) + + async def get_client(self, guild): """Get existing voice client for a guild.""" return discord.utils.get(self.bot.voice_clients, guild=guild) - async def get_voice_status(self, guild): - """Get voice client status including connection and playback state. + async def get_voice_client(self, guild): + """Backward-compatible alias for get_client().""" + return await self.get_client(guild) + + async def get_status(self, guild): + """Get voice client status. Returns: - tuple: (voice_client, is_connected, is_playing) or (None, False, False) if no client + tuple: (voice_client, is_connected, is_playing) or (None, False, False) """ - voice_client = await self.get_voice_client(guild) - if voice_client is None: + vc = await self.get_client(guild) + if vc is None: return None, False, False + return vc, vc.is_connected(), vc.is_playing() - is_connected = voice_client.is_connected() - is_playing = voice_client.is_playing() if is_connected else False + async def get_voice_status(self, guild): + """Backward-compatible alias for get_status().""" + return await self.get_status(guild) + + async def disconnect(self, guild): + """Stop playback and disconnect from voice.""" + vc = await self.get_client(guild) + if vc: + vc.stop() + logger.info("Disconnecting from voice") + await vc.disconnect() + logger.info(f"Disconnected from guild {guild}") - return voice_client, is_connected, is_playing + async def stop(self, guild): + """Backward-compatible alias for disconnect().""" + await self.disconnect(guild) - async def play_audio(self, channel, audio_source, volume=1.0): - """Play audio in a voice channel.""" - voice_client = await self.join_channel(channel) - if not voice_client: - return False + # --- Playback ------------------------------------------------------------ - if abs(volume - 1.0) > 1e-9: - audio_source = discord.PCMVolumeTransformer(audio_source) - audio_source.volume = volume + async def _play_await(self, vc, audio_source): + """Play an audio source and wait for completion using an asyncio.Future. + + Returns True on success, False on error. No polling, no magic sleeps. + """ + loop = asyncio.get_event_loop() + future = loop.create_future() + + def after(error): + if not future.done(): + if error: + future.set_exception(Exception(str(error))) + else: + future.set_result(True) + + vc.play(audio_source, after=after) try: - voice_client.play(audio_source) - while voice_client.is_playing(): - await asyncio.sleep(0.1) + await future return True except Exception as e: - logger.error(f"Error playing audio: {e}") + logger.error(f"Playback error: {e}") return False - async def stop(self, guild): - """Stop playing and disconnect from voice.""" - voice_client = await self.get_voice_client(guild) - if voice_client: - voice_client.stop() - logger.info("stop() called - disconnecting from voice") - await voice_client.disconnect() - logger.info(f"Stopped and disconnected from guild {guild}") + async def play_source(self, channel, audio_source, interrupt=True, volume=1.0): + """Connect to *channel* and play a pre-made audio source. + + Returns True on success, False on failure. + """ + vc = await self.connect(channel) + if not vc: + logger.error("play_source: could not connect to voice channel") + return False + if interrupt and (vc.is_playing() or vc.is_paused()): + vc.stop() -class Voice(commands.Cog): - def __init__(self, bot): - self.bot = bot - self._voice_lock = asyncio.Lock() + if abs(volume - 1.0) > 1e-9: + audio_source = discord.PCMVolumeTransformer(audio_source) + audio_source.volume = volume - async def _validate_user_voice_state(self, ctx): - if not ctx.author.voice or not ctx.author.voice.channel: - await ctx.send("You need to be in a voice channel first!") - return False - return True + logger.info("Playing via play_source") + return await self._play_await(vc, audio_source) - async def _create_voice_client(self, ctx): - async with self._voice_lock: - try: - await asyncio.sleep(0.5) - voice_client = await ctx.author.voice.channel.connect(timeout=30.0, reconnect=False) - logger.info(f"Connected to voice channel: {ctx.author.voice.channel}") - return voice_client - except asyncio.TimeoutError: - await ctx.send("Connection to voice channel timed out.") - return None - except discord.ClientException as e: - logger.error(f"Failed to connect to voice channel: {e}") - await ctx.send("Failed to connect to voice channel. Please try again.", ephemeral=True) - return None + # --- Source preparation (folded from AudioPlaybackManager) --------------- - async def _move_voice_client(self, voice_client, ctx): - try: - await voice_client.move_to(ctx.author.voice.channel) - logger.info(f"Moved to voice channel: {ctx.author.voice.channel}") - return voice_client - except asyncio.TimeoutError: - await ctx.send("Connection to voice channel timed out.") - return None - except discord.ClientException as e: - logger.error(f"Failed to move to voice channel: {e}") - await ctx.send("Failed to move to your voice channel. Please try again.", ephemeral=True) + @staticmethod + async def validate_voice_channel(ctx): + """Validate user is in a voice channel. Returns the channel or None.""" + if not ctx.author.voice or not ctx.author.voice.channel: + await ctx.respond( + "Musisz być na kanale głosowym, żebym mógł dołączyć!", ephemeral=True + ) return None + return ctx.author.voice.channel + + @staticmethod + async def prepare_file_source(source): + """Validate a local file and return (display_name, absolute_path) or (None, None).""" + logger.info(f"Attempting to play local file: {source}") + + if not os.path.isabs(source): + source = os.path.abspath(source) + logger.debug(f"Converted to absolute path: {source}") + + if not os.path.exists(source): + logger.error(f"File not found: {source}") + return None, None + + filename = os.path.basename(source) + display_name = os.path.splitext(filename)[0] + return display_name, source - async def _verify_connection(self, voice_client): - if not voice_client.is_connected(): + @staticmethod + async def prepare_audio_source(source, is_url, display_name): + """Prepare FFmpegPCMAudio from a URL or local file. + + Returns (audio_source, final_display_name). + """ + if is_url: + before_options = "-reconnect 1 -reconnect_streamed 1 -reconnect_delay_max 5" + audio_source = discord.FFmpegPCMAudio(source, before_options=before_options) + filename = os.path.basename(source.split("?")[0]) + filename = urllib.parse.unquote(filename) + clean_filename = os.path.splitext(filename)[0] + return audio_source, clean_filename + else: + audio_source = discord.FFmpegPCMAudio(source=source) try: - await voice_client.connect(reconnect=False, timeout=10.0) - return voice_client + metadata = mutagen.open(source) + if metadata: + title = ( + metadata.get("title", [None])[0] + if hasattr(metadata, "get") + else None + ) + if title: + display_name = title + logger.debug(f"Read metadata - title: {title}") + else: + display_name = os.path.splitext(os.path.basename(source))[0] except Exception as e: - logger.error(f"Failed to reconnect to voice: {e}") - return None - return voice_client + logger.warning(f"Could not read metadata from {source}: {str(e)}") + display_name = os.path.splitext(os.path.basename(source))[0] - async def ensure_voice_client(self, ctx): - if not await self._validate_user_voice_state(ctx): - return None + return audio_source, display_name - voice_client = discord.utils.get(self.bot.voice_clients, guild=ctx.guild) + @staticmethod + async def prepare_source(source): + """Determine whether *source* is a URL or local file. - if voice_client and not voice_client.is_connected(): + Returns (source_path, display_name, is_url) or (None, None, None) on failure. + """ + is_url = source.startswith(("http://", "https://")) + + if is_url: + logger.info(f"Attempting to play stream from URL: {source}") + return source, source, is_url + + display_name, absolute_source = await VoiceManager.prepare_file_source(source) + if not display_name: + return None, None, None + + return absolute_source, display_name, is_url + + @staticmethod + async def cleanup_temp_file(source): + """Remove a temporary file if it exists.""" + if source and os.path.exists(source): try: - logger.info("Voice client exists but not connected - disconnecting stale client") - await voice_client.disconnect() - except Exception: - pass - voice_client = None - - if not voice_client or not voice_client.is_connected(): - if not await self._validate_user_voice_state(ctx): - return None - return await self._create_voice_client(ctx) + os.remove(source) + logger.info(f"Cleaned up temporary file: {source}") + except Exception as e: + logger.error(f"Error cleaning up file {source}: {str(e)}") - if voice_client.channel != ctx.author.voice.channel: - return await self._move_voice_client(voice_client, ctx) + @staticmethod + async def cleanup_if_needed(source, delete_after_play, is_url): + """Conditionally clean up a file after playback.""" + if delete_after_play and not is_url: + await VoiceManager.cleanup_temp_file(source) - return await self._verify_connection(voice_client) + # --- High-level play (full flow) ----------------------------------------- - async def connect(self, ctx): - voice_client = await self.ensure_voice_client(ctx) - if not voice_client: - return None - return voice_client + async def play(self, ctx, source, delete_after_play=False, interrupt=True): + """High-level playback: validate context, prepare source, connect, play, cleanup. + + Args: + ctx: Discord application context (used for channel resolution & error messages). + source: Local file path or URL to stream. + delete_after_play: Remove local file after playback (no-op for URLs). + interrupt: Stop currently playing audio before starting. + + Returns: + bool: True if playback completed successfully, False otherwise. + """ + prepared_source, display_name, is_url = await VoiceManager.prepare_source(source) + if not prepared_source: + await VoiceManager.cleanup_if_needed(source, delete_after_play, False) + return False + + target_channel = await VoiceManager.validate_voice_channel(ctx) + if not target_channel: + await VoiceManager.cleanup_if_needed(prepared_source, delete_after_play, is_url) + return False + + vc = await self.connect(target_channel) + if not vc: + await VoiceManager.cleanup_if_needed(prepared_source, delete_after_play, is_url) + return False + + if interrupt and (vc.is_playing() or vc.is_paused()): + vc.stop() + + audio_source, final_display_name = await VoiceManager.prepare_audio_source( + prepared_source, is_url, display_name + ) + + success = await self._play_await(vc, audio_source) + + if success: + logger.info(f"Playback completed: {final_display_name}") + else: + logger.error(f"Playback failed: {prepared_source}") + if not success: + await VoiceManager.cleanup_if_needed(prepared_source, delete_after_play, is_url) -def setup(bot): - bot.add_cog(Voice(bot)) + return success diff --git a/main.py b/main.py index 8f75ef4..cab1f54 100644 --- a/main.py +++ b/main.py @@ -75,7 +75,6 @@ if __name__ == "__main__": "cogs.trigger", "cogs.cron", "cogs.qr", - "cogs.voice", "cogs.uploader", ] -- cgit v1.2.3 From 4663f260b55de0975661fda1c00f60d2ca6e4506 Mon Sep 17 00:00:00 2001 From: salmonik Date: Wed, 20 May 2026 15:40:18 +0200 Subject: Clean up consumers: replace polling with VoiceManager.play_source() - cron.py: replace while-is_playing polling with VoiceManager.play_source() - vclogs.py: replace while-is_playing polling with VoiceManager.play_source() - stop.py: use VoiceManager.disconnect() instead of raw voice_client - join.py: use VoiceManager.connect() instead of raw channel.connect() - voice.py: add back join_by_channel_id() convenience method - Tests: update for new VoiceManager method names and signatures --- cogs/cron.py | 26 ++++++++++---------------- cogs/music/join.py | 4 +++- cogs/music/stop.py | 5 ++++- cogs/vclogs.py | 7 +------ cogs/voice.py | 11 +++++++++++ tests/cogs/test_music_audio.py | 4 ++-- tests/cogs/test_music_join.py | 25 ++++++++++++++----------- tests/cogs/test_voice.py | 4 ++-- 8 files changed, 47 insertions(+), 39 deletions(-) (limited to 'cogs/voice.py') diff --git a/cogs/cron.py b/cogs/cron.py index aad43d2..ee88085 100644 --- a/cogs/cron.py +++ b/cogs/cron.py @@ -145,29 +145,23 @@ class Cron(commands.Cog): if isinstance(channel, discord.VoiceChannel): voice_mgr = get_voice_manager(self.bot) - voice_client = await voice_mgr.join_channel(channel) - if not voice_client: - if channel.guild.system_channel: - await channel.guild.system_channel.send( - "Nie udano się połączyć z kanałem głosowym" - ) - return try: audio_source = discord.FFmpegPCMAudio(audio_url) - voice_client.play(audio_source) + success = await voice_mgr.play_source(channel, audio_source) - while voice_client.is_playing(): - await asyncio.sleep(1) + if success: + logger.info(f"Finished playing audio: {audio_url}") + else: + raise Exception("Playback failed") - logger.info("Audio playback finished normally - disconnecting") - await voice_client.disconnect() - logger.info(f"Finished playing audio: {audio_url}") + await voice_mgr.disconnect(channel.guild) except Exception as e: logger.error(f"Error playing audio: {e}") - if voice_client.is_connected(): - logger.info("Audio playback failed - disconnecting") - await voice_client.disconnect() + try: + await voice_mgr.disconnect(channel.guild) + except Exception: + pass if channel.guild.system_channel: await channel.guild.system_channel.send( f"Błąd podczas odgrywania audio: {audio_url}" diff --git a/cogs/music/join.py b/cogs/music/join.py index c613b10..d68ca79 100644 --- a/cogs/music/join.py +++ b/cogs/music/join.py @@ -6,6 +6,7 @@ import logging from discord.ext import commands +from cogs.voice import get_voice_manager logger = logging.getLogger(__name__) @@ -16,8 +17,9 @@ class Join(commands.Cog): @commands.slash_command(name="join", description="dołącza do kanalu") async def join(self, ctx): + voice_mgr = get_voice_manager(self.bot) channel = ctx.author.voice.channel - await channel.connect() + await voice_mgr.connect(channel) await ctx.respond(content="\u200b", ephemeral=True) await ctx.interaction.delete_original_response() logger.info("Bot connected to voice channel") diff --git a/cogs/music/stop.py b/cogs/music/stop.py index f06e7f5..9c8d5cf 100644 --- a/cogs/music/stop.py +++ b/cogs/music/stop.py @@ -5,6 +5,8 @@ from discord.ext import commands +from cogs.voice import get_voice_manager + import logging logger = logging.getLogger(__name__) @@ -18,7 +20,8 @@ class Stop(commands.Cog): async def stop(self, ctx): await ctx.respond(content="\u200b", ephemeral=True) await ctx.interaction.delete_original_response() - await ctx.voice_client.disconnect() + voice_mgr = get_voice_manager(self.bot) + await voice_mgr.disconnect(ctx.guild) logger.info("Bot disconnected from voice channel") diff --git a/cogs/vclogs.py b/cogs/vclogs.py index b538265..2e853ad 100644 --- a/cogs/vclogs.py +++ b/cogs/vclogs.py @@ -160,12 +160,7 @@ class VcLogs(commands.Cog): logger.info("preparing to play audio") audio_source = discord.FFmpegPCMAudio(tts_file) - if volume != 1.0: - audio_source = discord.PCMVolumeTransformer(audio_source) - audio_source.volume = volume - voice_client.play(audio_source) - while voice_client.is_playing(): - await asyncio.sleep(0.1) + await voice_mgr.play_source(channel, audio_source, volume=volume) except Exception as e: logger.error(f"TTS playback error: {e}") finally: diff --git a/cogs/voice.py b/cogs/voice.py index ba1993e..86416ef 100644 --- a/cogs/voice.py +++ b/cogs/voice.py @@ -151,6 +151,17 @@ class VoiceManager: """Backward-compatible alias for connect().""" return await self.connect(channel) + async def join_by_channel_id(self, channel_id): + """Join a voice channel by channel ID.""" + channel = self.bot.get_channel(channel_id) + if not channel: + logger.error(f"Channel {channel_id} not found") + return None + if not isinstance(channel, discord.VoiceChannel): + logger.error(f"Channel {channel_id} is not a voice channel") + return None + return await self.connect(channel) + async def get_client(self, guild): """Get existing voice client for a guild.""" return discord.utils.get(self.bot.voice_clients, guild=guild) diff --git a/tests/cogs/test_music_audio.py b/tests/cogs/test_music_audio.py index 705f1b3..150ad5d 100644 --- a/tests/cogs/test_music_audio.py +++ b/tests/cogs/test_music_audio.py @@ -59,7 +59,7 @@ async def test_prepare_file_source_absolute_path(): async def test_prepare_source_url(): from cogs.music.audio_playback import AudioPlaybackManager - source, _, is_url = await AudioPlaybackManager.prepare_source(MagicMock(), "https://example.com/stream") + source, _, is_url = await AudioPlaybackManager.prepare_source("https://example.com/stream") assert is_url == True assert source == "https://example.com/stream" @@ -70,7 +70,7 @@ async def test_prepare_source_local_file(): from cogs.music.audio_playback import AudioPlaybackManager with patch('os.path.exists', return_value=True): - source, display_name, is_url = await AudioPlaybackManager.prepare_source(MagicMock(), "/tmp/test.mp3") + source, display_name, is_url = await AudioPlaybackManager.prepare_source("/tmp/test.mp3") assert is_url == False assert display_name == "test" diff --git a/tests/cogs/test_music_join.py b/tests/cogs/test_music_join.py index 0176759..b40d93a 100644 --- a/tests/cogs/test_music_join.py +++ b/tests/cogs/test_music_join.py @@ -24,16 +24,18 @@ async def test_join_command_success(join_cog, mock_channel, mock_guild, bot, moc ctx.author = mock_user mock_voice_channel = MagicMock() - mock_voice = MagicMock() - mock_voice_channel.connect = AsyncMock(return_value=mock_voice) - ctx.author.voice = MagicMock() ctx.author.voice.channel = mock_voice_channel - with patch.object(mock_voice_channel, 'connect', new_callable=AsyncMock, return_value=mock_voice): + with patch('cogs.music.join.get_voice_manager') as mock_get_vm: + mock_vm = MagicMock() + mock_vm.connect = AsyncMock() + mock_get_vm.return_value = mock_vm + with patch.object(ctx, 'respond', new_callable=AsyncMock): with patch.object(ctx.interaction, 'delete_original_response', new_callable=AsyncMock): await join_cog.join.callback(join_cog, ctx) + mock_vm.connect.assert_called_once_with(mock_voice_channel) @pytest.mark.asyncio @@ -42,12 +44,13 @@ async def test_stop_command_success(stop_cog, mock_channel, mock_guild, bot, moc ctx.user = mock_user ctx.author = mock_user - mock_voice_client = MagicMock() - mock_voice_client.disconnect = AsyncMock() - ctx.voice_client = mock_voice_client + with patch('cogs.music.stop.get_voice_manager') as mock_get_vm: + mock_vm = MagicMock() + mock_vm.disconnect = AsyncMock() + mock_get_vm.return_value = mock_vm - with patch.object(ctx, 'respond', new_callable=AsyncMock): - with patch.object(ctx.interaction, 'delete_original_response', new_callable=AsyncMock): - await stop_cog.stop.callback(stop_cog, ctx) + with patch.object(ctx, 'respond', new_callable=AsyncMock): + with patch.object(ctx.interaction, 'delete_original_response', new_callable=AsyncMock): + await stop_cog.stop.callback(stop_cog, ctx) - mock_voice_client.disconnect.assert_called_once() + mock_vm.disconnect.assert_called_once_with(mock_guild) diff --git a/tests/cogs/test_voice.py b/tests/cogs/test_voice.py index 104eea6..6eb226b 100644 --- a/tests/cogs/test_voice.py +++ b/tests/cogs/test_voice.py @@ -53,7 +53,7 @@ async def test_voice_manager_get_voice_status_not_connected(): mock_bot = MagicMock() mock_guild = MagicMock() - with patch('cogs.voice.VoiceManager.get_voice_client', return_value=None): + with patch('cogs.voice.VoiceManager.get_client', return_value=None): manager = VoiceManager(mock_bot) voice_client, is_connected, is_playing = await manager.get_voice_status(mock_guild) @@ -71,7 +71,7 @@ async def test_voice_manager_stop(): mock_voice_client = MagicMock() mock_voice_client.disconnect = AsyncMock() - with patch('cogs.voice.VoiceManager.get_voice_client', return_value=mock_voice_client): + with patch('cogs.voice.VoiceManager.get_client', return_value=mock_voice_client): manager = VoiceManager(mock_bot) await manager.stop(mock_guild) -- cgit v1.2.3 From 8d35930871db37b1009275c423ce95ed72954613 Mon Sep 17 00:00:00 2001 From: salmonik Date: Wed, 20 May 2026 17:08:40 +0200 Subject: Fix stale-connection reconnect hang and TTS temp-file collision - _ensure_voice_client: call _remove_voice_client() when cleaning up stale or disconnected clients so the fresh connect doesn't hang for 30s waiting for Discord's stale state to clear - Reduce connect timeout from 30s to 10s for faster failure recovery - vclogs.py: use a unique temp filename per TTS call (include timestamp) instead of relying on PID alone, preventing concurrent TTS calls from clobbering each other's files --- cogs/vclogs.py | 2 +- cogs/voice.py | 30 ++++++++++++++++++++++++++---- 2 files changed, 27 insertions(+), 5 deletions(-) (limited to 'cogs/voice.py') diff --git a/cogs/vclogs.py b/cogs/vclogs.py index 2e853ad..01a428d 100644 --- a/cogs/vclogs.py +++ b/cogs/vclogs.py @@ -116,7 +116,7 @@ class VcLogs(commands.Cog): voice_mgr = self.get_voice_manager() temp_dir = get_temp_dir() - tts_file = os.path.join(temp_dir, f"temp_tts_{os.getpid()}.mp3") + tts_file = os.path.join(temp_dir, f"temp_tts_{os.getpid()}_{asyncio.get_event_loop().time()}.mp3") tts = gTTS(text=text, lang='pl') logger.info("saving file") diff --git a/cogs/voice.py b/cogs/voice.py index 86416ef..f079c6a 100644 --- a/cogs/voice.py +++ b/cogs/voice.py @@ -84,6 +84,12 @@ class VoiceManager: await vc.disconnect() except Exception: pass + # Forcefully clear stale internal state so the fresh + # connect doesn't hang for 30s waiting for Discord. + try: + channel._state._remove_voice_client(guild.id) + except Exception: + pass vc = None else: try: @@ -103,6 +109,10 @@ class VoiceManager: await vc.disconnect() except Exception: pass + try: + channel._state._remove_voice_client(guild.id) + except Exception: + pass vc = None # --- create fresh connection -------------------------------------- @@ -110,7 +120,7 @@ class VoiceManager: try: await asyncio.sleep(0.3) try: - vc = await channel.connect(timeout=30.0, reconnect=False) + vc = await channel.connect(timeout=10.0, reconnect=False) logger.info(f"Connected to voice channel: {channel}") return vc except discord.ClientException as e: @@ -129,7 +139,7 @@ class VoiceManager: pass await asyncio.sleep(0.3) try: - vc = await channel.connect(timeout=30.0, reconnect=False) + vc = await channel.connect(timeout=10.0, reconnect=False) logger.info(f"Reconnected to voice channel: {channel}") return vc except Exception as e2: @@ -346,6 +356,9 @@ class VoiceManager: async def play(self, ctx, source, delete_after_play=False, interrupt=True): """High-level playback: validate context, prepare source, connect, play, cleanup. + Sends ``grom: {name}`` immediately on start, and an error follow-up + if playback fails. + Args: ctx: Discord application context (used for channel resolution & error messages). source: Local file path or URL to stream. @@ -377,14 +390,23 @@ class VoiceManager: prepared_source, is_url, display_name ) + try: + await ctx.followup.send(f"grom: {final_display_name}") + except Exception as e: + logger.error(e) + success = await self._play_await(vc, audio_source) if success: logger.info(f"Playback completed: {final_display_name}") else: logger.error(f"Playback failed: {prepared_source}") - - if not success: + try: + await ctx.followup.send( + "eror: Timeout - nie udało się połączyć ze streamem", ephemeral=True + ) + except Exception: + pass await VoiceManager.cleanup_if_needed(prepared_source, delete_after_play, is_url) return success -- cgit v1.2.3 From 35545e30303faa517a98ec18c660e5f6c063c912 Mon Sep 17 00:00:00 2001 From: salmonik Date: Thu, 21 May 2026 16:45:31 +0200 Subject: Fix voice reconnect race: reuse VoiceClient instead of destroying it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stale-connection path was calling _remove_voice_client() then creating a brand-new VoiceClient via channel.connect(). This created a race: VOICE_SERVER_UPDATE (triggered by Discord when the user joins voice) arrives between the destroy and the fresh-connect, gets consumed by the old client, and the new client never receives it — causing a 10s timeout waiting for got_both_voice_updates. Fix: after disconnect(), call vc.connect() on the same VoiceClient instance. The disconnect resets the state machine to 'disconnected' but the same VoiceConnectionState processes any events that arrive between disconnect and reconnect, so the state machine correctly reaches got_both_voice_updates. --- cogs/voice.py | 54 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 11 deletions(-) (limited to 'cogs/voice.py') diff --git a/cogs/voice.py b/cogs/voice.py index f079c6a..a224336 100644 --- a/cogs/voice.py +++ b/cogs/voice.py @@ -80,17 +80,29 @@ class VoiceManager: logger.info(f"Already connected to {channel}") return vc logger.warning(f"Stale connection to {channel}, reconnecting") + # Disconnect resets the state machine to 'disconnected' + # but keeps the VoiceClient alive. Reusing it avoids a + # race where Discord sends VOICE_SERVER_UPDATE between + # the destroy and the fresh-connect below, causing the + # new client to timeout waiting for the event (since the + # old one already consumed it). try: await vc.disconnect() except Exception: pass - # Forcefully clear stale internal state so the fresh - # connect doesn't hang for 30s waiting for Discord. try: - channel._state._remove_voice_client(guild.id) - except Exception: - pass - vc = None + await vc.connect( + reconnect=False, timeout=10.0, self_deaf=False, self_mute=False + ) + logger.info(f"Reconnected to voice channel: {channel}") + return vc + except Exception as e: + logger.error(f"Failed to reconnect stale voice client: {e}") + try: + channel._state._remove_voice_client(guild.id) + except Exception: + pass + vc = None else: try: await vc.move_to(channel) @@ -102,18 +114,38 @@ class VoiceManager: await vc.disconnect() except Exception: pass + try: + await vc.connect( + reconnect=False, timeout=10.0, self_deaf=False, self_mute=False + ) + logger.info(f"Reconnected after failed move: {channel}") + return vc + except Exception: + pass + try: + channel._state._remove_voice_client(guild.id) + except Exception: + pass vc = None else: - logger.info("Voice client exists but disconnected, cleaning up stale entry") + logger.info("Voice client exists but disconnected, reconnecting") try: await vc.disconnect() except Exception: pass try: - channel._state._remove_voice_client(guild.id) - except Exception: - pass - vc = None + await vc.connect( + reconnect=False, timeout=10.0, self_deaf=False, self_mute=False + ) + logger.info(f"Reconnected disconnected client: {channel}") + return vc + except Exception as e: + logger.error(f"Failed to reconnect disconnected client: {e}") + try: + channel._state._remove_voice_client(guild.id) + except Exception: + pass + vc = None # --- create fresh connection -------------------------------------- self._connecting.add(guild_id) -- cgit v1.2.3