From 57cdf847f5d97c72b9c8e6b04e17227db88d3046 Mon Sep 17 00:00:00 2001 From: TheOnlyMace <0815cracky@gmail.com> Date: Fri, 24 Jul 2026 21:49:20 +0200 Subject: [PATCH] Implement overdue giveaway recovery and enhance giveaway command handling - Added functionality to recover overdue giveaways during bot startup, ensuring that giveaways that have passed their end time are processed correctly. - Introduced `cancelGiveawayJob` to safely remove scheduled jobs, preventing issues with locked jobs. - Updated giveaway command handling to trim IDs and validate existence within the guild context, improving error handling and user feedback. - Enhanced the role picker component to normalize search queries and handle errors more gracefully, improving user experience. - Implemented caching strategies for guild roles to avoid stale or empty responses, ensuring more reliable data retrieval. --- apps/bot/src/index.ts | 15 ++++ apps/bot/src/modules/giveaways/commands.ts | 26 ++++++- apps/bot/src/modules/giveaways/index.ts | 2 +- apps/bot/src/modules/giveaways/service.ts | 74 +++++++++++++++++-- .../giveaways/[giveawayId]/action/route.ts | 9 ++- .../src/components/ui/discord-role-select.tsx | 49 ++++++++++-- apps/webui/src/hooks/use-guild-roles.ts | 38 ++++++++-- apps/webui/src/lib/discord-guild-resources.ts | 52 +++++++++++-- .../webui/src/lib/module-configs/giveaways.ts | 69 +++++++++++++---- docs/PHASE-TRACKING.md | 18 +++++ 10 files changed, 301 insertions(+), 51 deletions(-) diff --git a/apps/bot/src/index.ts b/apps/bot/src/index.ts index 25e7e1b..5757d55 100644 --- a/apps/bot/src/index.ts +++ b/apps/bot/src/index.ts @@ -116,6 +116,21 @@ if (!isShard) { logger.error({ error }, 'Failed to ensure recurring jobs'); captureException(error, { phase: 'ensureRecurringJobs' }); } + + try { + await runOncePerCluster( + 'nexumi:lock:recover-overdue-giveaways', + 60, + async () => { + const { recoverOverdueGiveaways } = await import('./modules/giveaways/index.js'); + await recoverOverdueGiveaways(context); + }, + 'Overdue giveaway recovery' + ); + } catch (error) { + logger.error({ error }, 'Failed to recover overdue giveaways'); + captureException(error, { phase: 'recoverOverdueGiveaways' }); + } } try { diff --git a/apps/bot/src/modules/giveaways/commands.ts b/apps/bot/src/modules/giveaways/commands.ts index bd28e59..58292ac 100644 --- a/apps/bot/src/modules/giveaways/commands.ts +++ b/apps/bot/src/modules/giveaways/commands.ts @@ -6,6 +6,7 @@ import { requirePermission } from '../../permissions.js'; import { parseDuration } from '../moderation/duration.js'; import { giveawayCommandData } from './command-definitions.js'; import { + cancelGiveawayJob, deleteGiveaway, endGiveaway, GiveawayError, @@ -100,14 +101,23 @@ const giveawayCommand: SlashCommand = { try { if (sub === 'end') { - const id = interaction.options.getString('id', true); + const id = interaction.options.getString('id', true).trim(); + const giveaway = await context.prisma.giveaway.findUnique({ where: { id } }); + if (!giveaway || giveaway.guildId !== interaction.guildId) { + throw new GiveawayError('not_found'); + } + await cancelGiveawayJob(id); await endGiveaway(context, id); await interaction.reply({ content: t(locale, 'giveaway.ended'), ephemeral: true }); return; } if (sub === 'reroll') { - const id = interaction.options.getString('id', true); + const id = interaction.options.getString('id', true).trim(); + const existing = await context.prisma.giveaway.findUnique({ where: { id } }); + if (!existing || existing.guildId !== interaction.guildId) { + throw new GiveawayError('not_found'); + } const updated = await rerollGiveaway(context, id, locale); await interaction.reply({ content: tf(locale, 'giveaway.rerolled', { @@ -136,14 +146,22 @@ const giveawayCommand: SlashCommand = { } if (sub === 'delete') { - const id = interaction.options.getString('id', true); + const id = interaction.options.getString('id', true).trim(); + const existing = await context.prisma.giveaway.findUnique({ where: { id } }); + if (!existing || existing.guildId !== interaction.guildId) { + throw new GiveawayError('not_found'); + } await deleteGiveaway(context, id); await interaction.reply({ content: t(locale, 'giveaway.deleted'), ephemeral: true }); return; } if (sub === 'pause') { - const id = interaction.options.getString('id', true); + const id = interaction.options.getString('id', true).trim(); + const existing = await context.prisma.giveaway.findUnique({ where: { id } }); + if (!existing || existing.guildId !== interaction.guildId) { + throw new GiveawayError('not_found'); + } const updated = await pauseGiveaway(context, id, locale); await interaction.reply({ content: t(locale, updated.paused ? 'giveaway.paused' : 'giveaway.resumed'), diff --git a/apps/bot/src/modules/giveaways/index.ts b/apps/bot/src/modules/giveaways/index.ts index 6423d4d..110a162 100644 --- a/apps/bot/src/modules/giveaways/index.ts +++ b/apps/bot/src/modules/giveaways/index.ts @@ -1,3 +1,3 @@ export { giveawayCommands } from './commands.js'; export { isGiveawayButton, handleGiveawayButton } from './buttons.js'; -export { endGiveaway, scheduleGiveawayEnd, runGiveawayCreateJob, GiveawayError } from './service.js'; +export { endGiveaway, scheduleGiveawayEnd, runGiveawayCreateJob, recoverOverdueGiveaways, GiveawayError } from './service.js'; diff --git a/apps/bot/src/modules/giveaways/service.ts b/apps/bot/src/modules/giveaways/service.ts index 449bd0a..e0d07d2 100644 --- a/apps/bot/src/modules/giveaways/service.ts +++ b/apps/bot/src/modules/giveaways/service.ts @@ -22,25 +22,46 @@ export class GiveawayError extends Error { } } +export function giveawayEndJobId(giveawayId: string): string { + return `giveaway-end-${giveawayId}`; +} + +/** + * Removes a scheduled/failed end job if present. Locked (active) jobs cannot be + * removed — callers must not invoke this from inside the giveawayEnd worker. + */ +export async function cancelGiveawayJob(giveawayId: string): Promise { + const job = await giveawayQueue.getJob(giveawayEndJobId(giveawayId)); + if (!job) { + return; + } + try { + await job.remove(); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + if (message.includes('locked') || message.includes('could not be removed')) { + logger.warn({ giveawayId, error }, 'Giveaway end job is locked; skipping remove'); + return; + } + throw error; + } +} + export async function scheduleGiveawayEnd(giveawayId: string, endsAt: Date): Promise { + await cancelGiveawayJob(giveawayId); const delay = Math.max(0, endsAt.getTime() - Date.now()); await giveawayQueue.add( 'giveawayEnd', { giveawayId }, { delay, - jobId: `giveaway-end-${giveawayId}`, + jobId: giveawayEndJobId(giveawayId), removeOnComplete: true, removeOnFail: 50 } ); } -export async function cancelGiveawayJob(giveawayId: string): Promise { - const job = await giveawayQueue.getJob(`giveaway-end-${giveawayId}`); - await job?.remove(); -} - function requirementLines(locale: 'de' | 'en', giveaway: Giveaway): string[] { const lines: string[] = []; if (giveaway.requiredRoleId) { @@ -316,6 +337,12 @@ async function dmWinners( } } +/** + * Draws winners and marks the giveaway ended. Does **not** cancel BullMQ jobs — + * the slash-command / dashboard paths cancel the delayed job first; the + * `giveawayEnd` worker must never remove its own locked job (that threw and + * left giveaways stuck with the join button still visible). + */ export async function endGiveaway(context: BotContext, giveawayId: string): Promise { const giveaway = await context.prisma.giveaway.findUnique({ where: { id: giveawayId } }); if (!giveaway) { @@ -325,7 +352,6 @@ export async function endGiveaway(context: BotContext, giveawayId: string): Prom throw new GiveawayError('already_ended'); } - await cancelGiveawayJob(giveawayId); const locale = await getGuildLocale(context.prisma, giveaway.guildId); const guild = await context.client.guilds.fetch(giveaway.guildId); const eligible = await filterEligibleEntrants(context, guild, giveaway); @@ -347,6 +373,40 @@ export async function endGiveaway(context: BotContext, giveawayId: string): Prom return updated; } +/** + * Ends giveaways whose `endsAt` has passed but `endedAt` is still null + * (e.g. after a failed/locked BullMQ job). Safe to call on bot ready. + */ +export async function recoverOverdueGiveaways(context: BotContext): Promise { + const overdue = await context.prisma.giveaway.findMany({ + where: { + endedAt: null, + endsAt: { lte: new Date() } + }, + take: 100, + orderBy: { endsAt: 'asc' } + }); + + let recovered = 0; + for (const giveaway of overdue) { + try { + await cancelGiveawayJob(giveaway.id); + await endGiveaway(context, giveaway.id); + recovered += 1; + } catch (error) { + if (error instanceof GiveawayError && error.code === 'already_ended') { + continue; + } + logger.error({ error, giveawayId: giveaway.id }, 'Failed to recover overdue giveaway'); + } + } + + if (recovered > 0) { + logger.info({ recovered }, 'Recovered overdue giveaways'); + } + return recovered; +} + export async function rerollGiveaway( context: BotContext, giveawayId: string, diff --git a/apps/webui/src/app/api/guilds/[guildId]/giveaways/[giveawayId]/action/route.ts b/apps/webui/src/app/api/guilds/[guildId]/giveaways/[giveawayId]/action/route.ts index 300ed0e..1e581d4 100644 --- a/apps/webui/src/app/api/guilds/[guildId]/giveaways/[giveawayId]/action/route.ts +++ b/apps/webui/src/app/api/guilds/[guildId]/giveaways/[giveawayId]/action/route.ts @@ -2,7 +2,11 @@ import { GiveawayActionSchema } from '@nexumi/shared'; import { type NextRequest, NextResponse } from 'next/server'; import { requireGuildAccess, toApiErrorResponse } from '@/lib/auth'; import { writeDashboardAudit } from '@/lib/audit'; -import { endGiveawayDashboard, setGiveawayPaused } from '@/lib/module-configs/giveaways'; +import { + endGiveawayDashboard, + GiveawayEndError, + setGiveawayPaused +} from '@/lib/module-configs/giveaways'; import { prisma } from '@/lib/prisma'; interface RouteParams { @@ -39,6 +43,9 @@ export async function POST(request: NextRequest, { params }: RouteParams) { return NextResponse.json(giveaway); } catch (error) { + if (error instanceof GiveawayEndError) { + return NextResponse.json({ error: error.message }, { status: 502 }); + } return toApiErrorResponse(error); } } diff --git a/apps/webui/src/components/ui/discord-role-select.tsx b/apps/webui/src/components/ui/discord-role-select.tsx index 1a56242..3f59969 100644 --- a/apps/webui/src/components/ui/discord-role-select.tsx +++ b/apps/webui/src/components/ui/discord-role-select.tsx @@ -26,6 +26,10 @@ function toOptions(roles: DiscordRoleOption[]) { })); } +function normalizeSearch(value: string): string { + return value.trim().toLocaleLowerCase(); +} + interface RoleSelectBaseProps { guildId: string; disabled?: boolean; @@ -48,14 +52,33 @@ export function DiscordRoleSelect({ allowClear = true }: DiscordRoleSelectProps) { const t = useTranslations(); - const { roles, loading } = useGuildRoles(guildId); + const { roles, loading, error, reload } = useGuildRoles(guildId); const [open, setOpen] = useState(false); + const [query, setQuery] = useState(''); const options = useMemo(() => toOptions(roles), [roles]); const selected = options.find((option) => option.id === value); + const filtered = useMemo(() => { + const needle = normalizeSearch(query); + if (!needle) { + return options; + } + return options.filter((option) => { + const haystack = normalizeSearch(`${option.prefix}${option.name} ${option.id}`); + return haystack.includes(needle); + }); + }, [options, query]); return ( - + { + setOpen(next); + if (!next) { + setQuery(''); + } + }} + > + ) : ( + t('common.noResults') + )} + {allowClear && value ? ( {t('common.clearSelection')} ) : null} - {options.map((option) => ( + {filtered.map((option) => ( { onChange(option.id); setOpen(false); diff --git a/apps/webui/src/hooks/use-guild-roles.ts b/apps/webui/src/hooks/use-guild-roles.ts index 2b77c28..fb1e5d7 100644 --- a/apps/webui/src/hooks/use-guild-roles.ts +++ b/apps/webui/src/hooks/use-guild-roles.ts @@ -1,44 +1,68 @@ 'use client'; import type { DiscordRoleOption } from '@nexumi/shared'; -import { useEffect, useState } from 'react'; +import { useCallback, useEffect, useState } from 'react'; const cache = new Map(); export function useGuildRoles(guildId: string): { roles: DiscordRoleOption[]; loading: boolean; + error: boolean; + reload: () => void; } { const [roles, setRoles] = useState(() => cache.get(guildId) ?? []); const [loading, setLoading] = useState(!cache.has(guildId)); + const [error, setError] = useState(false); + const [reloadToken, setReloadToken] = useState(0); + + const reload = useCallback(() => { + cache.delete(guildId); + setReloadToken((token) => token + 1); + }, [guildId]); useEffect(() => { let cancelled = false; - const cached = cache.get(guildId); + const cached = reloadToken === 0 ? cache.get(guildId) : undefined; if (cached) { setRoles(cached); setLoading(false); + setError(false); return; } setLoading(true); + setError(false); void fetch(`/api/guilds/${guildId}/roles`) .then(async (response) => { if (!response.ok) { - return [] as DiscordRoleOption[]; + throw new Error(`Roles request failed with ${response.status}`); } - return (await response.json()) as DiscordRoleOption[]; + const data = (await response.json()) as DiscordRoleOption[]; + if (!Array.isArray(data)) { + throw new Error('Roles response was not an array'); + } + return data; }) .then((data) => { if (cancelled) { return; } - cache.set(guildId, data); + // Only cache successful non-empty payloads so a one-off failure + // cannot stick the picker on "Keine Treffer". + if (data.length > 0) { + cache.set(guildId, data); + } else { + cache.delete(guildId); + } setRoles(data); + setError(false); }) .catch(() => { if (!cancelled) { + cache.delete(guildId); setRoles([]); + setError(true); } }) .finally(() => { @@ -50,7 +74,7 @@ export function useGuildRoles(guildId: string): { return () => { cancelled = true; }; - }, [guildId]); + }, [guildId, reloadToken]); - return { roles, loading }; + return { roles, loading, error, reload }; } diff --git a/apps/webui/src/lib/discord-guild-resources.ts b/apps/webui/src/lib/discord-guild-resources.ts index e67b836..b90bd70 100644 --- a/apps/webui/src/lib/discord-guild-resources.ts +++ b/apps/webui/src/lib/discord-guild-resources.ts @@ -45,11 +45,38 @@ interface DiscordRoleRest { managed?: boolean; } +async function readCache(cacheKey: string): Promise { + try { + if (redis.status === 'wait') { + await redis.connect(); + } + const cached = await redis.get(cacheKey); + if (!cached) { + return null; + } + return JSON.parse(cached) as T; + } catch { + // Cache is best-effort; fall through to Discord API. + return null; + } +} + +async function writeCache(cacheKey: string, value: unknown): Promise { + try { + if (redis.status === 'wait') { + await redis.connect(); + } + await redis.set(cacheKey, JSON.stringify(value), 'EX', CACHE_TTL_SECONDS); + } catch { + // ignore cache write failures + } +} + export async function listGuildChannelsForDashboard(guildId: string): Promise { const cacheKey = `dashboard:guild:${guildId}:channels:v2`; - const cached = await redis.get(cacheKey); + const cached = await readCache(cacheKey); if (cached) { - return JSON.parse(cached) as DiscordChannelOption[]; + return cached; } const channels = await discordBotFetch(`/guilds/${guildId}/channels`); @@ -63,18 +90,23 @@ export async function listGuildChannelsForDashboard(guildId: string): Promise a.name.localeCompare(b.name)); - await redis.set(cacheKey, JSON.stringify(options), 'EX', CACHE_TTL_SECONDS); + await writeCache(cacheKey, options); return options; } export async function listGuildRolesForDashboard(guildId: string): Promise { - const cacheKey = `dashboard:guild:${guildId}:roles`; - const cached = await redis.get(cacheKey); - if (cached) { - return JSON.parse(cached) as DiscordRoleOption[]; + // v2: invalidate any stale empty/corrupt cache entries from earlier bugs + const cacheKey = `dashboard:guild:${guildId}:roles:v2`; + const cached = await readCache(cacheKey); + if (cached && cached.length > 0) { + return cached; } const roles = await discordBotFetch(`/guilds/${guildId}/roles`); + if (!Array.isArray(roles)) { + throw new Error(`Discord API returned non-array roles for guild ${guildId}`); + } + const options = roles .filter((role) => role.name !== '@everyone') .map((role) => ({ @@ -85,7 +117,11 @@ export async function listGuildRolesForDashboard(guildId: string): Promise b.position - a.position); - await redis.set(cacheKey, JSON.stringify(options), 'EX', CACHE_TTL_SECONDS); + // Never cache an empty list — that freezes "Keine Treffer" for the TTL + // after a transient Discord/API glitch. + if (options.length > 0) { + await writeCache(cacheKey, options); + } return options; } diff --git a/apps/webui/src/lib/module-configs/giveaways.ts b/apps/webui/src/lib/module-configs/giveaways.ts index 1ce2629..60dbfc0 100644 --- a/apps/webui/src/lib/module-configs/giveaways.ts +++ b/apps/webui/src/lib/module-configs/giveaways.ts @@ -9,6 +9,13 @@ export class GiveawayCreateTimeoutError extends Error { } } +export class GiveawayEndError extends Error { + constructor(message: string) { + super(message); + this.name = 'GiveawayEndError'; + } +} + function toDashboard(giveaway: Giveaway): GiveawayDashboard { return { id: giveaway.id, @@ -30,6 +37,26 @@ function toDashboard(giveaway: Giveaway): GiveawayDashboard { }; } +function scheduledEndJobId(giveawayId: string): string { + return `giveaway-end-${giveawayId}`; +} + +async function safeRemoveJob(jobId: string): Promise { + const job = await getGiveawayQueue().getJob(jobId); + if (!job) { + return; + } + try { + await job.remove(); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + if (message.includes('locked') || message.includes('could not be removed')) { + return; + } + throw error; + } +} + export async function listGiveaways(guildId: string): Promise { const giveaways = await prisma.giveaway.findMany({ where: { guildId }, @@ -102,9 +129,9 @@ export async function setGiveawayPaused( } /** - * Ends a giveaway immediately by re-scheduling its `giveawayEnd` job with no - * delay and waiting for the bot to draw winners, update the Discord message - * and DM them - see `apps/bot/src/modules/giveaways/service.ts#endGiveaway`. + * Ends a giveaway immediately by enqueueing a one-shot `giveawayEnd` job + * (unique jobId so it never collides with the scheduled/failed timer job) + * and waiting for the bot to draw winners. */ export async function endGiveawayDashboard(guildId: string, giveawayId: string): Promise { const existing = await prisma.giveaway.findFirst({ where: { id: giveawayId, guildId } }); @@ -112,21 +139,32 @@ export async function endGiveawayDashboard(guildId: string, giveawayId: string): return null; } - const existingJob = await getGiveawayQueue().getJob(`giveaway-end-${giveawayId}`); - await existingJob?.remove(); + await safeRemoveJob(scheduledEndJobId(giveawayId)); - const { confirmed } = await addJobAndAwait( - getGiveawayQueue(), - getGiveawayQueueEvents(), - 'giveawayEnd', - { giveawayId }, - { jobId: `giveaway-end-${giveawayId}`, removeOnComplete: true, removeOnFail: 50, delay: 0 }, - 30_000 - ); + // Unique jobId: reusing `giveaway-end-${id}` fails when a failed/locked + // timer job still occupies that id in Redis. + const manualJobId = `giveaway-end-now-${giveawayId}-${Date.now()}`; + + let confirmed = false; + try { + const result = await addJobAndAwait( + getGiveawayQueue(), + getGiveawayQueueEvents(), + 'giveawayEnd', + { giveawayId }, + { jobId: manualJobId, removeOnComplete: true, removeOnFail: 50, delay: 0 }, + 30_000 + ); + confirmed = result.confirmed; + } catch (error) { + throw new GiveawayEndError( + error instanceof Error ? error.message : 'Failed to enqueue giveaway end job' + ); + } const updated = await prisma.giveaway.findUnique({ where: { id: giveawayId } }); if (!updated?.endedAt) { - throw new Error( + throw new GiveawayEndError( confirmed ? 'Giveaway end job finished without marking the giveaway as ended' : 'Giveaway end timed out – the bot may still be processing; refresh and retry' @@ -141,8 +179,7 @@ export async function deleteGiveawayDashboard(guildId: string, giveawayId: strin return false; } - const job = await getGiveawayQueue().getJob(`giveaway-end-${giveawayId}`); - await job?.remove(); + await safeRemoveJob(scheduledEndJobId(giveawayId)); await prisma.giveaway.delete({ where: { id: giveawayId } }); return true; diff --git a/docs/PHASE-TRACKING.md b/docs/PHASE-TRACKING.md index cfc67fc..54415c5 100644 --- a/docs/PHASE-TRACKING.md +++ b/docs/PHASE-TRACKING.md @@ -272,6 +272,24 @@ - [ ] `/giveaway end` mit falscher ID → Fehlermeldung; mit korrekter ID → Reroll möglich - [ ] Dashboard „Jetzt beenden“ → Status beendet, Discord-Nachricht aktualisiert +## Post-Phase – Giveaway End Fix + Role Picker (Status: implementiert) + +### Abgeschlossen (Code) + +- Root cause: `endGiveaway` removed its own locked BullMQ job → End-Job crashte vor DB-Update (Timer/UI/Command hingen) +- `cancelGiveawayJob` nur noch außerhalb des Workers (Command/Dashboard); locked remove wird ignoriert +- Dashboard-End nutzt eindeutige Job-IDs (`giveaway-end-now-…`); klarere 502-Fehler statt generischem 500 +- Bot-Start: `recoverOverdueGiveaways` beendet überfällige aktive Giveaways +- Role-Picker: kein Cache von leeren/fehlerhaften Responses; Redis-Cache best-effort; manuelle Filter statt cmdk-Default + +### Manuell testen + +- [ ] Giveaway starten (kurz), warten bis Timer abläuft → Embed „Beendet“, Button weg, Gewinner gezogen +- [ ] Giveaway vorzeitig per Dashboard „Jetzt beenden“ → kein Internal Server Error +- [ ] `/giveaway list` → ID kopieren → `/giveaway end id:…` → Erfolg +- [ ] Verifizierung: Rollen-Dropdown zeigt Serverrollen (nicht nur „Keine Treffer“) +- [ ] Nach Deploy: bereits hängende Giveaways werden beim Bot-Start recovered + ## Post-Phase – Starboard Verbesserungen (Status: implementiert) ### Abgeschlossen (Code)