discord-tickets / bot

The most popular open-source and self-hosted ticket management bot for Discord - a free alternative to the premium and white-label plans of other popular ticketing bots.
https://discordtickets.app
GNU General Public License v3.0
903 stars 473 forks source link

[BUG] BETA V4 - Tickets ID duplicating #418

Closed Spectrum2k closed 1 year ago

Spectrum2k commented 1 year ago

Is there an existing issue for this?

Current Behavior

If users create many tickets at one time, tickets are created with the same ID, and only one ticket can work (other tickets give errors during any interactions).

Expected Behavior

Tickets IDs should not be duplicated.

Steps To Reproduce

  1. Run v4 bot
  2. 2 (or more) peoples creates tickets in one moment

Environment

- OS: Docker node:18-alpine (Docker on Ubuntu 22.04)
- Node: 18
- NPM: -
- Bot: latest v4

Anything else?

Logs:

bot-bot-1    |  05/05/23 11:13:26  [INFO] (BUTTONS) CyberBobeks#7777 used the "create" button
bot-bot-1    |  05/05/23 11:13:28  [INFO] (BUTTONS) Spectrum2k#2517 used the "create" button
bot-bot-1    |  05/05/23 11:13:39  [INFO] (MODALS) CyberBobeks#7777 used the "topic" modal
bot-bot-1    |  05/05/23 11:13:40  [INFO] (MODALS) Spectrum2k#2517 used the "topic" modal
bot-bot-1    |  05/05/23 11:13:41  [INFO] (TICKETS) CyberBobeks#7777 created ticket 1104002974386233374
bot-bot-1    |  05/05/23 11:13:42  [ERROR] DiscordAPIError[50013]: Missing Permissions
bot-bot-1    |     at SequentialHandler.runRequest (/usr/bot/node_modules/.pnpm/@discordjs+rest@1.6.0/node_modules/@discordjs/rest/dist/index.js:933:15)
bot-bot-1    |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
bot-bot-1    |     at async SequentialHandler.queueRequest (/usr/bot/node_modules/.pnpm/@discordjs+rest@1.6.0/node_modules/@discordjs/rest/dist/index.js:712:14)
bot-bot-1    |     at async REST.request (/usr/bot/node_modules/.pnpm/@discordjs+rest@1.6.0/node_modules/@discordjs/rest/dist/index.js:1321:22)
bot-bot-1    |     at async Guild.fetchAuditLogs (/usr/bot/node_modules/.pnpm/discord.js@14.8.0_bufferutil@4.0.7_utf-8-validate@5.0.10/node_modules/discord.js/src/structures/Guild.js:733:18)
bot-bot-1    |     at async module.exports.run (/usr/bot/src/listeners/client/messageDelete.js:30:21) {
bot-bot-1    |   rawError: { message: 'Missing Permissions', code: 50013 },
bot-bot-1    |   code: 50013,
bot-bot-1    |   status: 403,
bot-bot-1    |   method: 'GET',
bot-bot-1    |   url: 'https://discord.com/api/v10/guilds/1001431814373642321/audit-logs?limit=1&action_type=72',
bot-bot-1    |   requestBody: { files: undefined, json: undefined }
bot-bot-1    | }
bot-bot-1    |  05/05/23 11:13:42  [WARN] (TICKETS) An error occurred whilst creating ticket 1104002976235917414
bot-bot-1    |  05/05/23 11:13:42  [ERROR] (TICKETS) 61c11909-ede4-408c-9db0-72442c50d10b
bot-bot-1    |  05/05/23 11:13:42  [ERROR] (TICKETS) PrismaClientKnownRequestError: 
bot-bot-1    | Invalid `prisma.ticket.create()` invocation:
bot-bot-1    | 
bot-bot-1    | 
bot-bot-1    | Unique constraint failed on the constraint: `tickets_guildId_number_key`
bot-bot-1    |     at Zr.handleRequestError (/usr/bot/node_modules/.pnpm/@prisma+client@4.11.0_prisma@4.11.0/node_modules/@prisma/client/runtime/library.js:171:6414)
bot-bot-1    |     at Zr.handleAndLogRequestError (/usr/bot/node_modules/.pnpm/@prisma+client@4.11.0_prisma@4.11.0/node_modules/@prisma/client/runtime/library.js:171:5948)
bot-bot-1    |     at Zr.request (/usr/bot/node_modules/.pnpm/@prisma+client@4.11.0_prisma@4.11.0/node_modules/@prisma/client/runtime/library.js:171:5786)
bot-bot-1    |     at async t._request (/usr/bot/node_modules/.pnpm/@prisma+client@4.11.0_prisma@4.11.0/node_modules/@prisma/client/runtime/library.js:174:10455)
bot-bot-1    |     at async TicketManager.postQuestions (/usr/bot/src/lib/tickets/manager.js:635:19)
bot-bot-1    |     at async TopicModal.run (/usr/bot/src/modals/topic.js:95:4) {
bot-bot-1    |   code: 'P2002',
bot-bot-1    |   clientVersion: '4.11.0',
bot-bot-1    |   meta: { target: 'tickets_guildId_number_key' }
bot-bot-1    | }

Interaction on "bad" ticket:

bot-bot-1    |  05/05/23 11:16:13  [INFO] (BUTTONS) Spectrum2k#2517 used the "edit" button
bot-bot-1    |  05/05/23 11:16:13  [ERROR] (BUTTONS) 6f9961c0-a6a3-4320-9fea-34d2912449f4
bot-bot-1    |  05/05/23 11:16:13  [ERROR] (BUTTONS) "edit" button execution error: TypeError: Cannot read properties of null (reading 'guild')
bot-bot-1    |     at EditButton.run (/usr/bot/src/buttons/edit.js:36:51)
M4rlus commented 1 year ago

@eartharoid maybe use uuids for tickets and remove the ticket number completely Edit: I looked it up, the best solution seems like to add a secondary key with auto increment. Otherwhise you have to make it a non async function, that would result in waiting issues with the bot.

RooRay commented 1 year ago

@eartharoid maybe use uuids for tickets and remove the ticket number completely

Edit:

I looked it up, the best solution seems like to add a secondary key with auto increment.

Otherwhise you have to make it a non async function, that would result in waiting issues with the bot.

Or just queue up requests so the bot responds to them one at a time, Discords API latency which is fairly high in comparison to the bots latency means that the bot could spend an extra bit doing that and the average user likely won't notice

eartharoid commented 1 year ago

maybe use uuids for tickets and remove the ticket number completely

Tickets already have a unique ID as the primary key, it's the channel ID. The number is a secondary more user-friendly ID.

I looked it up, the best solution seems like to add a secondary key with auto increment

As far as I can tell, it's impossible to use AUTO_INCREMENT in a way that would allow guilds to have their own counter.

Or just queue up requests so the bot responds to them one at a time, Discords API latency which is fairly high in comparison to the bots latency means that the bot could spend an extra bit doing that and the average user likely won't notice

I thought of this too. It could be possible to create a queue for each guild but I'm not sure how to do that, especially in a way that doesn't negatively impact performance more than necessary. It would need only to affect the postQuestions function, and the bot would need to defer the reply before queueing the interaction; It already takes undesirably long for the bot to reply with the "Your ticket has been created" message, so I don't want to make it any slower.

I also thought about transactions/locking, but this would need to block all reads and writes on the tickets table, which I'm not sure how to do, and that would definitely slow things down.

There's also OCC, but I don't see how that can work in this situation.

I think a simpler solution is possible:

https://github.com/discord-tickets/bot/blob/cf93f085ac4309ba1da24341b13b4ddbada5b2a4/src/lib/tickets/manager.js#L376

https://github.com/discord-tickets/bot/blob/cf93f085ac4309ba1da24341b13b4ddbada5b2a4/src/lib/tickets/manager.js#L640

There are currently over 250 lines between reading and writing with several Promises being awaited in between. The second could be moved to just after the first, and use a non-locking transaction to update the row with the rest of the data (the channel, opening message etc). This would require changing the primary key from channelId to (guildId, number). It would (probably, I don't know if it actually makes a difference) be even better if the two queries were combined,

INSERT INTO tickets (
  number
)
VALUES (
  SELECT number FROM (SELECT MAX(number) + 1 AS number FROM tickets WHERE guildId = '$guildId') AS T
);

This should work, but I don't think it's possible in Prisma without using $queryRaw.

Alternatively, just catch the error and retry, although that's not very elegant as the channel name will be wrong.

M4rlus commented 1 year ago

my 30 second reply: just remove the id thing completely and use the rich presence for ticket count xd bc there is no way to make this nice and without errors or feature removal.

either you remove it completely or you try to fix it which will cause performance issues or otherwhise you remove the function to let the user set a starting id

eartharoid commented 1 year ago

Loading and storing the ticket count for each guild at startup like member and category counts is a good option.