From 4b17dfedc35470e504f5d84ec372e6c990b85c1d Mon Sep 17 00:00:00 2001 From: Valerie Date: Mon, 26 May 2025 03:59:48 -0400 Subject: [PATCH] Refactor Leaderboard cog to improve session management and error handling. Initialize aiohttp session in setup, enhance admin secret validation, and provide clearer error messages for API interactions. Update leaderboard command responses for better user feedback and handling of empty data scenarios. --- leaderboard/leaderboard.py | 96 +++++++++++++++++++++++++++++--------- 1 file changed, 74 insertions(+), 22 deletions(-) diff --git a/leaderboard/leaderboard.py b/leaderboard/leaderboard.py index 77abde4..076f60c 100644 --- a/leaderboard/leaderboard.py +++ b/leaderboard/leaderboard.py @@ -15,7 +15,7 @@ class Leaderboard(commands.Cog): def __init__(self, bot: Red): self.bot = bot self.config = Config.get_conf(self, identifier=867530999, force_registration=True) - self.session = aiohttp.ClientSession() + self.session = None # Initialize in setup self.api_base_url = "https://ruby.valerie.lol/api" self.admin_secret = None self.cache_time = 300 # 5 minutes cache @@ -33,17 +33,29 @@ class Leaderboard(commands.Cog): self.config.register_guild(**default_guild) def cog_unload(self): - asyncio.create_task(self.session.close()) + if self.session: + asyncio.create_task(self.session.close()) async def initialize(self): - """Load the admin secret from bot config.""" + """Load the admin secret from bot config and initialize session.""" self.admin_secret = await self.bot.get_shared_api_tokens("ruby_api") if not self.admin_secret.get("admin_secret"): - log.error("No admin secret found. Leaderboard functionality will be limited.") + log.error("No admin secret found. Please set it using [p]set api ruby_api admin_secret,") + return False + + # Initialize aiohttp session + if not self.session: + self.session = aiohttp.ClientSession() + return True async def _get_leaderboard(self) -> Optional[list]: """Fetch the global leaderboard from the API.""" - if not self.admin_secret: + if not self.admin_secret or not self.admin_secret.get("admin_secret"): + log.error("Admin secret not configured") + return None + + if not self.session: + log.error("Session not initialized") return None try: @@ -59,19 +71,33 @@ class Leaderboard(commands.Cog): ) as resp: if resp.status == 200: data = await resp.json() - self._cache["leaderboard"] = data.get("leaderboard", []) + if not isinstance(data, dict) or "leaderboard" not in data: + log.error("Invalid API response format") + return None + self._cache["leaderboard"] = data["leaderboard"] self._last_update["leaderboard"] = now return self._cache["leaderboard"] - else: - log.error(f"Failed to fetch leaderboard: {resp.status}") + elif resp.status == 401: + log.error("Unauthorized: Invalid admin secret") return None + else: + log.error(f"Failed to fetch leaderboard: Status {resp.status}") + return None + except aiohttp.ClientError as e: + log.error(f"API connection error: {e}") + return None except Exception as e: - log.error(f"Error fetching leaderboard: {e}") + log.error(f"Unexpected error fetching leaderboard: {e}") return None async def _update_points(self, user_id: str, username: str, points: int) -> bool: """Update a user's points in the global leaderboard.""" - if not self.admin_secret: + if not self.admin_secret or not self.admin_secret.get("admin_secret"): + log.error("Admin secret not configured") + return False + + if not self.session: + log.error("Session not initialized") return False try: @@ -87,16 +113,29 @@ class Leaderboard(commands.Cog): "points": points } ) as resp: - return resp.status == 200 + if resp.status == 200: + # Clear cache to ensure fresh data on next fetch + self._cache.pop("leaderboard", None) + return True + elif resp.status == 401: + log.error("Unauthorized: Invalid admin secret") + return False + else: + log.error(f"Failed to update points: Status {resp.status}") + return False + except aiohttp.ClientError as e: + log.error(f"API connection error: {e}") + return False except Exception as e: - log.error(f"Error updating points: {e}") + log.error(f"Unexpected error updating points: {e}") return False @commands.group(name="globalboard", aliases=["glb"]) async def globalboard(self, ctx: commands.Context): """Global leaderboard commands.""" if ctx.invoked_subcommand is None: - await ctx.send_help(ctx.command) + # Don't send help here, just show the base command response + await ctx.send("Use `!help globalboard` to see available commands.") @globalboard.command(name="show") async def show_leaderboard(self, ctx: commands.Context, page: int = 1): @@ -105,12 +144,20 @@ class Leaderboard(commands.Cog): leaderboard_data = await self._get_leaderboard() if not leaderboard_data: - return await ctx.send("Failed to fetch leaderboard data.") + if not self.admin_secret or not self.admin_secret.get("admin_secret"): + return await ctx.send("Leaderboard is not configured. Please contact the bot administrator.") + return await ctx.send("Failed to fetch leaderboard data. Please try again later.") + + if not leaderboard_data: # Empty leaderboard + return await ctx.send("The leaderboard is currently empty!") items_per_page = 10 pages = [leaderboard_data[i:i + items_per_page] for i in range(0, len(leaderboard_data), items_per_page)] + if not pages: # No data after pagination + return await ctx.send("The leaderboard is currently empty!") + if not 1 <= page <= len(pages): return await ctx.send(f"Invalid page number. Please choose between 1 and {len(pages)}.") @@ -121,10 +168,13 @@ class Leaderboard(commands.Cog): start_pos = (page - 1) * items_per_page for i, entry in enumerate(entries, start=start_pos + 1): - username = entry["username"] - points = entry["points"] + username = entry.get("username", "Unknown User") + points = entry.get("points", 0) lines.append(f"{i}. {username}: {points:,} points") + if not lines: # No valid entries + return await ctx.send("No valid leaderboard entries found.") + header = f"🏆 Global Leaderboard (Page {page}/{len(pages)})" footer = f"Use {ctx.prefix}globalboard show to view other pages" @@ -138,21 +188,23 @@ class Leaderboard(commands.Cog): leaderboard_data = await self._get_leaderboard() if not leaderboard_data: - return await ctx.send("Failed to fetch leaderboard data.") + if not self.admin_secret or not self.admin_secret.get("admin_secret"): + return await ctx.send("Leaderboard is not configured. Please contact the bot administrator.") + return await ctx.send("Failed to fetch leaderboard data. Please try again later.") user_data = next( - (entry for entry in leaderboard_data if entry["userId"] == str(member.id)), + (entry for entry in leaderboard_data if entry.get("userId") == str(member.id)), None ) if user_data: rank = next( (i for i, entry in enumerate(leaderboard_data, 1) - if entry["userId"] == str(member.id)), + if entry.get("userId") == str(member.id)), None ) await ctx.send( - f"🏆 **{member.display_name}** has **{user_data['points']:,}** points " + f"🏆 **{member.display_name}** has **{user_data.get('points', 0):,}** points " f"(Rank: #{rank:,})" ) else: @@ -189,11 +241,11 @@ class Leaderboard(commands.Cog): if leaderboard_data: user_data = next( - (entry for entry in leaderboard_data if entry["userId"] == str(message.author.id)), + (entry for entry in leaderboard_data if entry.get("userId") == str(message.author.id)), None ) if user_data: - current_points = user_data["points"] + current_points = user_data.get("points", 0) # Update points new_points = current_points + points_per_message