From c155c94ea2fdd4ac1506fd05910dfdc7f51d8d5d Mon Sep 17 00:00:00 2001 From: MythEclipse Date: Thu, 21 May 2026 15:44:17 +0700 Subject: [PATCH] feat: enhance booster role functionality with error handling and database migration --- src/db/migrations/0001_misty_wraith.sql | 2 + src/db/migrations/meta/0001_snapshot.json | 123 ++++++++++++++++++++++ src/db/migrations/meta/_journal.json | 7 ++ src/db/schema.ts | 6 +- src/discord/interactionHandler.test.ts | 12 +++ src/discord/interactionHandler.ts | 16 ++- src/services/boosterRoleService.test.ts | 23 ++++ src/services/boosterRoleService.ts | 65 ++++++++---- src/services/discordRoleRepository.ts | 9 +- 9 files changed, 238 insertions(+), 25 deletions(-) create mode 100644 src/db/migrations/0001_misty_wraith.sql create mode 100644 src/db/migrations/meta/0001_snapshot.json diff --git a/src/db/migrations/0001_misty_wraith.sql b/src/db/migrations/0001_misty_wraith.sql new file mode 100644 index 0000000..947fbe2 --- /dev/null +++ b/src/db/migrations/0001_misty_wraith.sql @@ -0,0 +1,2 @@ +ALTER TABLE "booster_roles" ALTER COLUMN "created_at" SET DATA TYPE bigint;--> statement-breakpoint +ALTER TABLE "booster_roles" ALTER COLUMN "updated_at" SET DATA TYPE bigint; \ No newline at end of file diff --git a/src/db/migrations/meta/0001_snapshot.json b/src/db/migrations/meta/0001_snapshot.json new file mode 100644 index 0000000..a2d776f --- /dev/null +++ b/src/db/migrations/meta/0001_snapshot.json @@ -0,0 +1,123 @@ +{ + "id": "66a1346c-4510-4ec6-a055-a3d0bcf803c3", + "prevId": "e654a614-04fe-4432-aee1-659bc4a0475e", + "version": "7", + "dialect": "postgresql", + "tables": { + "public.booster_roles": { + "name": "booster_roles", + "schema": "", + "columns": { + "guild_id": { + "name": "guild_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "role_id": { + "name": "role_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "color": { + "name": "color", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "icon": { + "name": "icon", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "bigint", + "primaryKey": false, + "notNull": true + }, + "updated_at": { + "name": "updated_at", + "type": "bigint", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "booster_roles_guild_user_idx": { + "name": "booster_roles_guild_user_idx", + "columns": [ + { + "expression": "guild_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + }, + "booster_roles_guild_role_idx": { + "name": "booster_roles_guild_role_idx", + "columns": [ + { + "expression": "guild_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "role_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + } + }, + "enums": {}, + "schemas": {}, + "sequences": {}, + "roles": {}, + "policies": {}, + "views": {}, + "_meta": { + "columns": {}, + "schemas": {}, + "tables": {} + } +} \ No newline at end of file diff --git a/src/db/migrations/meta/_journal.json b/src/db/migrations/meta/_journal.json index e7809fc..d0c0655 100644 --- a/src/db/migrations/meta/_journal.json +++ b/src/db/migrations/meta/_journal.json @@ -8,6 +8,13 @@ "when": 1779350490110, "tag": "0000_simple_titania", "breakpoints": true + }, + { + "idx": 1, + "version": "7", + "when": 1779352840357, + "tag": "0001_misty_wraith", + "breakpoints": true } ] } \ No newline at end of file diff --git a/src/db/schema.ts b/src/db/schema.ts index 7d2265c..fa81497 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -1,4 +1,4 @@ -import { integer, pgTable, text, uniqueIndex } from "drizzle-orm/pg-core"; +import { bigint, pgTable, text, uniqueIndex } from "drizzle-orm/pg-core"; export const boosterRoles = pgTable( "booster_roles", @@ -9,8 +9,8 @@ export const boosterRoles = pgTable( name: text("name").notNull(), color: text("color"), icon: text("icon"), - createdAt: integer("created_at").notNull(), - updatedAt: integer("updated_at").notNull() + createdAt: bigint("created_at", { mode: "number" }).notNull(), + updatedAt: bigint("updated_at", { mode: "number" }).notNull() }, (table) => ({ userIdx: uniqueIndex("booster_roles_guild_user_idx").on(table.guildId, table.userId), diff --git a/src/discord/interactionHandler.test.ts b/src/discord/interactionHandler.test.ts index 8cb483c..52e2ff1 100644 --- a/src/discord/interactionHandler.test.ts +++ b/src/discord/interactionHandler.test.ts @@ -120,4 +120,16 @@ describe("handleInteraction", () => { expect(interaction.replies[0]).toEqual({ content: "Role name is already used", flags: MessageFlags.Ephemeral }); }); + + test("hides failed query details from user replies", async () => { + const interaction = new FakeInteraction("claim", { name: "VIP" }); + const service = new FakeService(); + service.claimRole = async () => { + throw new Error("Failed query: insert into booster_roles params: secret"); + }; + + await handleInteraction(interaction, service, { isBoosting: async () => true }); + + expect(interaction.replies[0]).toEqual({ content: "Failed to save booster role. Any created role was cleaned up. Try again.", flags: MessageFlags.Ephemeral }); + }); }); diff --git a/src/discord/interactionHandler.ts b/src/discord/interactionHandler.ts index 6cc4510..59a2ed7 100644 --- a/src/discord/interactionHandler.ts +++ b/src/discord/interactionHandler.ts @@ -95,10 +95,24 @@ export async function handleInteraction( throw new Error("Unknown booster-role subcommand"); } catch (error) { logger.warn("Booster-role command failed", { error }); - await interaction.reply({ content: error instanceof Error ? error.message : "Command failed", flags: MessageFlags.Ephemeral }); + await interaction.reply({ content: toUserErrorMessage(error), flags: MessageFlags.Ephemeral }); } } +function toUserErrorMessage(error: unknown): string { + if (!(error instanceof Error)) return "Command failed"; + + if (error.message.includes("Failed query")) { + return "Failed to save booster role. Any created role was cleaned up. Try again."; + } + + if (error.message.includes("Missing Permissions")) { + return "Bot is missing permissions or role position to manage this role."; + } + + return error.message; +} + function requireGuildId(guildId: string | null): string { if (!guildId) throw new Error("This command can only be used in a server"); return guildId; diff --git a/src/services/boosterRoleService.test.ts b/src/services/boosterRoleService.test.ts index 5b741b4..c362e8e 100644 --- a/src/services/boosterRoleService.test.ts +++ b/src/services/boosterRoleService.test.ts @@ -17,10 +17,17 @@ class MemoryRoleStore { } } +class FailingCreateRoleStore extends MemoryRoleStore { + async create(): Promise { + throw new Error("Database insert failed"); + } +} + class FakeRoleRepository implements RoleRepository { roles = new Map(); deletedRoleIds: string[] = []; assignedRoles: Array<{ userId: string; roleId: string }> = []; + removedRoles: Array<{ userId: string; roleId: string }> = []; constructor(initialRoles = [{ id: "existing-vip", name: "VIP", permissions: [], position: 1, color: null }]) { for (const role of initialRoles) { @@ -48,6 +55,10 @@ class FakeRoleRepository implements RoleRepository { this.assignedRoles.push({ userId, roleId }); } + async removeRole(userId: string, roleId: string) { + this.removedRoles.push({ userId, roleId }); + } + async deleteRole(roleId: string) { this.deletedRoleIds.push(roleId); this.roles.delete(roleId); @@ -130,4 +141,16 @@ describe("BoosterRoleService", () => { expect(roles.deletedRoleIds).toEqual([claimed.roleId]); expect(await store.findByUser("guild", "user")).toBeNull(); }); + + test("rolls back created and assigned role when storing claim fails", async () => { + const roles = new FakeRoleRepository([]); + const service = new BoosterRoleService(new FailingCreateRoleStore(), roles, { anchorPosition: 10 }); + + await expect(service.claimRole({ guildId: "guild", userId: "user", name: "First Role", color: null, isBoosting: true })).rejects.toThrow("Database insert failed"); + + expect(roles.assignedRoles).toEqual([{ userId: "user", roleId: "created-1" }]); + expect(roles.removedRoles).toEqual([{ userId: "user", roleId: "created-1" }]); + expect(roles.deletedRoleIds).toEqual(["created-1"]); + expect(roles.roles.has("created-1")).toBe(false); + }); }); diff --git a/src/services/boosterRoleService.ts b/src/services/boosterRoleService.ts index 50e6e06..bd7a1a7 100644 --- a/src/services/boosterRoleService.ts +++ b/src/services/boosterRoleService.ts @@ -29,6 +29,7 @@ export type RoleRepository = { createRole(input: { name: string; color: string | null; permissions: string[]; position: number }): Promise<{ id: string }>; updateRole(roleId: string, input: { name?: string; color?: string | null; icon?: string | null }): Promise; assignRole(userId: string, roleId: string): Promise; + removeRole(userId: string, roleId: string): Promise; deleteRole(roleId: string): Promise; }; @@ -79,27 +80,35 @@ export class BoosterRoleService { assertRolePositionIsSafe(position, this.options.anchorPosition); const role = await this.roles.createRole({ name, color, permissions: [], position }); - if (input.icon) { - this.validateRoleIcon(input.icon); - await this.roles.updateRole(role.id, { icon: input.icon.dataUri }); + let assigned = false; + + try { + if (input.icon) { + this.validateRoleIcon(input.icon); + await this.roles.updateRole(role.id, { icon: input.icon.dataUri }); + } + + await this.roles.assignRole(userId, role.id); + assigned = true; + + const timestamp = this.now(); + const record = { + guildId, + userId, + roleId: role.id, + name, + color, + icon: input.icon?.dataUri ?? null, + createdAt: timestamp, + updatedAt: timestamp + }; + + await this.store.create(record); + return record; + } catch (error) { + await this.rollbackClaim({ guildId, userId, roleId: role.id, assigned }); + throw error; } - - await this.roles.assignRole(userId, role.id); - - const timestamp = this.now(); - const record = { - guildId, - userId, - roleId: role.id, - name, - color, - icon: input.icon?.dataUri ?? null, - createdAt: timestamp, - updatedAt: timestamp - }; - - await this.store.create(record); - return record; } async renameRole(input: { guildId: string; userId: string; name: string }): Promise { @@ -139,6 +148,15 @@ export class BoosterRoleService { await this.store.delete(guildId, userId); } + private async rollbackClaim(input: { guildId: string; userId: string; roleId: string; assigned: boolean }): Promise { + if (input.assigned) { + await ignoreRollbackError(() => this.roles.removeRole(input.userId, input.roleId)); + } + + await ignoreRollbackError(() => this.roles.deleteRole(input.roleId)); + await ignoreRollbackError(() => this.store.delete(input.guildId, input.userId)); + } + private validateRoleIcon(icon: RoleIcon): void { if (!icon.contentType.startsWith("image/")) { throw new Error("Role icon must be an image"); @@ -157,3 +175,10 @@ export class BoosterRoleService { return record; } } + +async function ignoreRollbackError(action: () => Promise): Promise { + try { + await action(); + } catch { + } +} diff --git a/src/services/discordRoleRepository.ts b/src/services/discordRoleRepository.ts index 5aa074d..9c58ff8 100644 --- a/src/services/discordRoleRepository.ts +++ b/src/services/discordRoleRepository.ts @@ -2,7 +2,9 @@ import type { ColorResolvable, Guild, Role } from "discord.js"; import type { RoleRepository } from "./boosterRoleService"; export class DiscordRoleRepository implements RoleRepository { - constructor(private readonly guild: Guild, private readonly anchorRoleId: string | null) {} + constructor(private readonly guild: Guild, anchorRoleId: string | null) { + void anchorRoleId; + } async listRoles() { await this.guild.roles.fetch(); @@ -34,6 +36,11 @@ export class DiscordRoleRepository implements RoleRepository { await member.roles.add(roleId); } + async removeRole(userId: string, roleId: string): Promise { + const member = await this.guild.members.fetch(userId); + await member.roles.remove(roleId); + } + async deleteRole(roleId: string): Promise { const role = await this.fetchRole(roleId); await role.delete();