Fix @requires_team decorator API error handling
The @requires_team() decorator was incorrectly treating API errors as "no team" because get_user_team() was catching all exceptions and returning None. Changes: 1. get_user_team() now propagates exceptions instead of catching them - Allows callers to distinguish between "no team found" vs "API error" - Updated docstring to document the exception behavior 2. @requires_team() decorator now has try-except block - Returns specific error for "no team" (None result) - Returns helpful error for API/network issues (exception caught) - Logs exceptions for debugging 3. league_admin_only() decorator enhanced - Now supports both slash commands (Interaction) and prefix commands (Context) - Unified error handling for both command types 4. team_service.py and related updates - Team model field name corrected: team_abbrev -> abbrev This fixes the regression where /cc-create was failing with "no team" error when it should have been showing an API error message or working correctly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
bb82e56355
commit
0c001b6dab
@ -308,11 +308,13 @@ class AdminCommands(commands.Cog):
|
|||||||
Prefix command version of admin-sync for bootstrap scenarios.
|
Prefix command version of admin-sync for bootstrap scenarios.
|
||||||
|
|
||||||
Use this when slash commands aren't synced yet and you can't access /admin-sync.
|
Use this when slash commands aren't synced yet and you can't access /admin-sync.
|
||||||
|
Syncs to the current guild only (for multi-bot scenarios).
|
||||||
"""
|
"""
|
||||||
self.logger.info(f"Prefix command !admin-sync invoked by {ctx.author} in {ctx.guild}")
|
self.logger.info(f"Prefix command !admin-sync invoked by {ctx.author} in {ctx.guild}")
|
||||||
|
|
||||||
try:
|
try:
|
||||||
synced_commands = await self.bot.tree.sync()
|
# Sync to current guild only (not globally) for multi-bot scenarios
|
||||||
|
synced_commands = await self.bot.tree.sync(guild=ctx.guild)
|
||||||
|
|
||||||
embed = EmbedTemplate.create_base_embed(
|
embed = EmbedTemplate.create_base_embed(
|
||||||
title="✅ Commands Synced Successfully",
|
title="✅ Commands Synced Successfully",
|
||||||
@ -332,12 +334,13 @@ class AdminCommands(commands.Cog):
|
|||||||
embed.add_field(
|
embed.add_field(
|
||||||
name="Sync Details",
|
name="Sync Details",
|
||||||
value=f"**Total Commands:** {len(synced_commands)}\n"
|
value=f"**Total Commands:** {len(synced_commands)}\n"
|
||||||
|
f"**Sync Type:** Local Guild\n"
|
||||||
f"**Guild ID:** {ctx.guild.id}\n"
|
f"**Guild ID:** {ctx.guild.id}\n"
|
||||||
f"**Time:** {discord.utils.utcnow().strftime('%H:%M:%S UTC')}",
|
f"**Time:** {discord.utils.utcnow().strftime('%H:%M:%S UTC')}",
|
||||||
inline=False
|
inline=False
|
||||||
)
|
)
|
||||||
|
|
||||||
embed.set_footer(text="💡 Use /admin-sync (slash command) for future syncs")
|
embed.set_footer(text="💡 Use /admin-sync local:True for guild-only sync")
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
self.logger.error(f"Prefix command sync failed: {e}", exc_info=True)
|
self.logger.error(f"Prefix command sync failed: {e}", exc_info=True)
|
||||||
|
|||||||
@ -13,8 +13,10 @@ version: '3.8'
|
|||||||
|
|
||||||
services:
|
services:
|
||||||
discord-bot:
|
discord-bot:
|
||||||
# Pull image from Docker Hub
|
# Build locally from Dockerfile
|
||||||
image: manticorum67/major-domo-discordapp:latest
|
build:
|
||||||
|
context: .
|
||||||
|
dockerfile: Dockerfile
|
||||||
container_name: major-domo-discord-bot-v2
|
container_name: major-domo-discord-bot-v2
|
||||||
|
|
||||||
# Restart policy
|
# Restart policy
|
||||||
@ -24,10 +26,10 @@ services:
|
|||||||
env_file:
|
env_file:
|
||||||
- .env
|
- .env
|
||||||
|
|
||||||
# Production environment configuration
|
# Environment configuration (uses .env file values)
|
||||||
environment:
|
environment:
|
||||||
- LOG_LEVEL=${LOG_LEVEL:-INFO}
|
- LOG_LEVEL=${LOG_LEVEL:-INFO}
|
||||||
- ENVIRONMENT=production
|
- ENVIRONMENT=${ENVIRONMENT:-production}
|
||||||
- TESTING=${TESTING:-false}
|
- TESTING=${TESTING:-false}
|
||||||
- REDIS_URL=${REDIS_URL:-}
|
- REDIS_URL=${REDIS_URL:-}
|
||||||
- REDIS_CACHE_TTL=${REDIS_CACHE_TTL:-300}
|
- REDIS_CACHE_TTL=${REDIS_CACHE_TTL:-300}
|
||||||
@ -40,6 +42,10 @@ services:
|
|||||||
# Logs directory (persistent) - mounted to /app/logs where the application expects it
|
# Logs directory (persistent) - mounted to /app/logs where the application expects it
|
||||||
- ${LOGS_HOST_PATH:-./logs}:/app/logs:rw
|
- ${LOGS_HOST_PATH:-./logs}:/app/logs:rw
|
||||||
|
|
||||||
|
# Development volumes for local testing
|
||||||
|
- ../dev-logs:/app/dev-logs:rw
|
||||||
|
- ../dev-storage:/app/dev-storage:rw
|
||||||
|
|
||||||
# Network configuration
|
# Network configuration
|
||||||
networks:
|
networks:
|
||||||
- major-domo-network
|
- major-domo-network
|
||||||
@ -49,7 +55,6 @@ services:
|
|||||||
test: ["CMD", "python", "-c", "import sys; sys.exit(0)"]
|
test: ["CMD", "python", "-c", "import sys; sys.exit(0)"]
|
||||||
interval: 60s
|
interval: 60s
|
||||||
timeout: 10s
|
timeout: 10s
|
||||||
start-period: 30s
|
|
||||||
retries: 3
|
retries: 3
|
||||||
|
|
||||||
# Resource limits (production)
|
# Resource limits (production)
|
||||||
|
|||||||
@ -74,35 +74,34 @@ class TeamService(BaseService[Team]):
|
|||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
List of Team instances owned by the user, optionally filtered by type
|
List of Team instances owned by the user, optionally filtered by type
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
Exception: If there's an error communicating with the API
|
||||||
|
Allows caller to distinguish between "no teams" vs "error occurred"
|
||||||
"""
|
"""
|
||||||
try:
|
season = season or get_config().sba_current_season
|
||||||
season = season or get_config().sba_current_season
|
params = [
|
||||||
params = [
|
('owner_id', str(owner_id)),
|
||||||
('owner_id', str(owner_id)),
|
('season', str(season))
|
||||||
('season', str(season))
|
]
|
||||||
]
|
|
||||||
|
|
||||||
teams = await self.get_all_items(params=params)
|
teams = await self.get_all_items(params=params)
|
||||||
|
|
||||||
# Filter by roster type if specified
|
# Filter by roster type if specified
|
||||||
if roster_type and teams:
|
if roster_type and teams:
|
||||||
try:
|
try:
|
||||||
target_type = RosterType(roster_type)
|
target_type = RosterType(roster_type)
|
||||||
teams = [team for team in teams if team.roster_type() == target_type]
|
teams = [team for team in teams if team.roster_type() == target_type]
|
||||||
logger.debug(f"Filtered to {len(teams)} {roster_type} teams for owner {owner_id}")
|
logger.debug(f"Filtered to {len(teams)} {roster_type} teams for owner {owner_id}")
|
||||||
except ValueError:
|
except ValueError:
|
||||||
logger.warning(f"Invalid roster_type '{roster_type}' - returning all teams")
|
logger.warning(f"Invalid roster_type '{roster_type}' - returning all teams")
|
||||||
|
|
||||||
if teams:
|
if teams:
|
||||||
logger.debug(f"Found {len(teams)} teams for owner {owner_id} in season {season}")
|
logger.debug(f"Found {len(teams)} teams for owner {owner_id} in season {season}")
|
||||||
return teams
|
return teams
|
||||||
|
|
||||||
logger.debug(f"No teams found for owner {owner_id} in season {season}")
|
logger.debug(f"No teams found for owner {owner_id} in season {season}")
|
||||||
return []
|
return []
|
||||||
|
|
||||||
except Exception as e:
|
|
||||||
logger.error(f"Error getting teams for owner {owner_id}: {e}")
|
|
||||||
return []
|
|
||||||
|
|
||||||
@cached_single_item(ttl=1800) # 30-minute cache
|
@cached_single_item(ttl=1800) # 30-minute cache
|
||||||
async def get_team_by_owner(self, owner_id: int, season: Optional[int] = None) -> Optional[Team]:
|
async def get_team_by_owner(self, owner_id: int, season: Optional[int] = None) -> Optional[Team]:
|
||||||
|
|||||||
@ -36,7 +36,11 @@ async def get_user_team(user_id: int) -> Optional[dict]:
|
|||||||
user_id: Discord user ID
|
user_id: Discord user ID
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Team data dict if user has a team, None otherwise
|
Team data dict if user has a team, None if user has no team
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
Exception: If there's an error communicating with the API (network, timeout, etc.)
|
||||||
|
Allows caller to distinguish between "no team" vs "error checking"
|
||||||
|
|
||||||
Note:
|
Note:
|
||||||
The underlying service method uses @cached_single_item decorator,
|
The underlying service method uses @cached_single_item decorator,
|
||||||
@ -46,30 +50,25 @@ async def get_user_team(user_id: int) -> Optional[dict]:
|
|||||||
# Import here to avoid circular imports
|
# Import here to avoid circular imports
|
||||||
from services.team_service import team_service
|
from services.team_service import team_service
|
||||||
|
|
||||||
try:
|
# Get team by owner (Discord user ID)
|
||||||
# Get team by owner (Discord user ID)
|
# This call is automatically cached by TeamService
|
||||||
# This call is automatically cached by TeamService
|
config = get_config()
|
||||||
config = get_config()
|
team = await team_service.get_team_by_owner(
|
||||||
team = await team_service.get_team_by_owner(
|
owner_id=user_id,
|
||||||
owner_id=user_id,
|
season=config.sba_current_season
|
||||||
season=config.sba_current_season
|
)
|
||||||
)
|
|
||||||
|
|
||||||
if team:
|
if team:
|
||||||
logger.debug(f"User {user_id} has team: {team.lname}")
|
logger.debug(f"User {user_id} has team: {team.lname}")
|
||||||
return {
|
return {
|
||||||
'id': team.id,
|
'id': team.id,
|
||||||
'name': team.lname,
|
'name': team.lname,
|
||||||
'abbrev': team.team_abbrev,
|
'abbrev': team.abbrev,
|
||||||
'season': team.season
|
'season': team.season
|
||||||
}
|
}
|
||||||
|
|
||||||
logger.debug(f"User {user_id} does not have a team")
|
logger.debug(f"User {user_id} does not have a team")
|
||||||
return None
|
return None
|
||||||
|
|
||||||
except Exception as e:
|
|
||||||
logger.error(f"Error checking user team: {e}", exc_info=True)
|
|
||||||
return None
|
|
||||||
|
|
||||||
|
|
||||||
def is_league_server(guild_id: int) -> bool:
|
def is_league_server(guild_id: int) -> bool:
|
||||||
@ -127,22 +126,35 @@ def requires_team():
|
|||||||
def decorator(func: Callable) -> Callable:
|
def decorator(func: Callable) -> Callable:
|
||||||
@wraps(func)
|
@wraps(func)
|
||||||
async def wrapper(self, interaction: discord.Interaction, *args, **kwargs):
|
async def wrapper(self, interaction: discord.Interaction, *args, **kwargs):
|
||||||
# Check if user has a team
|
try:
|
||||||
team = await get_user_team(interaction.user.id)
|
# Check if user has a team
|
||||||
|
team = await get_user_team(interaction.user.id)
|
||||||
|
|
||||||
if team is None:
|
if team is None:
|
||||||
|
await interaction.response.send_message(
|
||||||
|
"❌ This command requires you to have a team in the SBa league. Contact an admin if you believe this is an error.",
|
||||||
|
ephemeral=True
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
|
# Store team info in interaction for command to use
|
||||||
|
# This allows commands to access the team without another lookup
|
||||||
|
interaction.extras['user_team'] = team
|
||||||
|
|
||||||
|
return await func(self, interaction, *args, **kwargs)
|
||||||
|
|
||||||
|
except Exception as e:
|
||||||
|
# Log the error for debugging
|
||||||
|
logger.error(f"Error checking team ownership for user {interaction.user.id}: {e}", exc_info=True)
|
||||||
|
|
||||||
|
# Provide helpful error message to user
|
||||||
await interaction.response.send_message(
|
await interaction.response.send_message(
|
||||||
"❌ This command requires you to have a team in the SBa league. Contact an admin if you believe this is an error.",
|
"❌ Unable to verify team ownership due to a temporary error. Please try again in a moment. "
|
||||||
|
"If this persists, contact an admin.",
|
||||||
ephemeral=True
|
ephemeral=True
|
||||||
)
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
# Store team info in interaction for command to use
|
|
||||||
# This allows commands to access the team without another lookup
|
|
||||||
interaction.extras['user_team'] = team
|
|
||||||
|
|
||||||
return await func(self, interaction, *args, **kwargs)
|
|
||||||
|
|
||||||
return wrapper
|
return wrapper
|
||||||
return decorator
|
return decorator
|
||||||
|
|
||||||
@ -215,47 +227,61 @@ def league_admin_only():
|
|||||||
"""
|
"""
|
||||||
Decorator requiring both league server AND admin permissions.
|
Decorator requiring both league server AND admin permissions.
|
||||||
|
|
||||||
Usage:
|
Works with BOTH slash commands (Interaction) and prefix commands (Context).
|
||||||
|
|
||||||
|
Usage (slash):
|
||||||
@discord.app_commands.command(name="force-sync")
|
@discord.app_commands.command(name="force-sync")
|
||||||
@league_admin_only()
|
@league_admin_only()
|
||||||
async def force_sync(self, interaction: discord.Interaction):
|
async def force_sync(self, interaction: discord.Interaction):
|
||||||
# Only league server admins can use this
|
# Only league server admins can use this
|
||||||
|
|
||||||
|
Usage (prefix):
|
||||||
|
@commands.command(name="admin-sync")
|
||||||
|
@league_admin_only()
|
||||||
|
async def admin_sync_prefix(self, ctx: commands.Context):
|
||||||
|
# Only league server admins can use this
|
||||||
"""
|
"""
|
||||||
def decorator(func: Callable) -> Callable:
|
def decorator(func: Callable) -> Callable:
|
||||||
@wraps(func)
|
@wraps(func)
|
||||||
async def wrapper(self, interaction: discord.Interaction, *args, **kwargs):
|
async def wrapper(self, ctx_or_interaction, *args, **kwargs):
|
||||||
|
# Detect if this is a Context (prefix) or Interaction (slash)
|
||||||
|
is_prefix = isinstance(ctx_or_interaction, commands.Context)
|
||||||
|
|
||||||
|
if is_prefix:
|
||||||
|
ctx = ctx_or_interaction
|
||||||
|
guild = ctx.guild
|
||||||
|
author = ctx.author
|
||||||
|
|
||||||
|
async def send_error(msg: str):
|
||||||
|
await ctx.send(msg)
|
||||||
|
else:
|
||||||
|
interaction = ctx_or_interaction
|
||||||
|
guild = interaction.guild
|
||||||
|
author = interaction.user
|
||||||
|
|
||||||
|
async def send_error(msg: str):
|
||||||
|
await interaction.response.send_message(msg, ephemeral=True)
|
||||||
|
|
||||||
# Check guild
|
# Check guild
|
||||||
if not interaction.guild:
|
if not guild:
|
||||||
await interaction.response.send_message(
|
await send_error("❌ This command can only be used in a server.")
|
||||||
"❌ This command can only be used in a server.",
|
|
||||||
ephemeral=True
|
|
||||||
)
|
|
||||||
return
|
return
|
||||||
|
|
||||||
# Check if league server
|
# Check if league server
|
||||||
if not is_league_server(interaction.guild.id):
|
if not is_league_server(guild.id):
|
||||||
await interaction.response.send_message(
|
await send_error("❌ This command is only available in the SBa league server.")
|
||||||
"❌ This command is only available in the SBa league server.",
|
|
||||||
ephemeral=True
|
|
||||||
)
|
|
||||||
return
|
return
|
||||||
|
|
||||||
# Check admin permissions
|
# Check admin permissions
|
||||||
if not isinstance(interaction.user, discord.Member):
|
if not isinstance(author, discord.Member):
|
||||||
await interaction.response.send_message(
|
await send_error("❌ Unable to verify permissions.")
|
||||||
"❌ Unable to verify permissions.",
|
|
||||||
ephemeral=True
|
|
||||||
)
|
|
||||||
return
|
return
|
||||||
|
|
||||||
if not interaction.user.guild_permissions.administrator:
|
if not author.guild_permissions.administrator:
|
||||||
await interaction.response.send_message(
|
await send_error("❌ This command requires administrator permissions.")
|
||||||
"❌ This command requires administrator permissions.",
|
|
||||||
ephemeral=True
|
|
||||||
)
|
|
||||||
return
|
return
|
||||||
|
|
||||||
return await func(self, interaction, *args, **kwargs)
|
return await func(self, ctx_or_interaction, *args, **kwargs)
|
||||||
|
|
||||||
return wrapper
|
return wrapper
|
||||||
return decorator
|
return decorator
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user