-
-
Notifications
You must be signed in to change notification settings - Fork 8
fix: prevent invalid UUIDs in chat message saves #715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { chats, messages, users } from '@/lib/db/schema'; | |
| import { eq, desc, and, sql, asc } from 'drizzle-orm'; // Added asc | ||
| import { alias } from 'drizzle-orm/pg-core'; | ||
| import { getCurrentUserIdOnServer } from '@/lib/auth/get-current-user'; // We'll use this to ensure user-specific actions | ||
| import { isValidUUID, generateUUID } from '@/lib/utils'; | ||
|
|
||
| // Define types based on our schema for better type safety | ||
| // These would ideally be generated by Drizzle Kit or defined in a central types location in a larger app | ||
|
|
@@ -12,6 +13,38 @@ export type User = typeof users.$inferSelect; | |
| export type NewChat = typeof chats.$inferInsert; | ||
| export type NewMessage = typeof messages.$inferInsert; | ||
|
|
||
| /** | ||
| * Normalizes a message ID to a valid UUID. | ||
| * If the ID is already valid, returns it as-is. | ||
| * If the ID contains invalid suffixes (e.g., "-mxb7wz"), strips them. | ||
| * If the ID is null/undefined or cannot be normalized, generates a fresh UUID. | ||
| * @param id - The message ID to normalize. | ||
| * @returns A valid UUID string. | ||
| */ | ||
| function normalizeMessageId(id: string | undefined | null): string { | ||
| // If no ID provided, generate a fresh one | ||
| if (!id) { | ||
| return generateUUID(); | ||
| } | ||
|
|
||
| // If already valid, return as-is | ||
| if (isValidUUID(id)) { | ||
| return id; | ||
| } | ||
|
|
||
| // Attempt to strip common suffixes (e.g., "-mxb7wz", "-zg8y3u") | ||
| // These are base62-encoded random strings appended by the old deduplication logic | ||
| const stripped = id.replace(/-[a-zA-Z0-9]{4,10}$/, ''); | ||
|
|
||
| if (isValidUUID(stripped)) { | ||
| return stripped; | ||
| } | ||
|
|
||
| // If we can't salvage it, generate a fresh UUID | ||
| console.warn(`[chat-db] Invalid UUID detected and regenerated: "${id}"`); | ||
| return generateUUID(); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves a specific chat by its ID, ensuring it belongs to the current user | ||
| * or is public. | ||
|
|
@@ -73,9 +106,12 @@ export async function getChatsPage( | |
|
|
||
| /** | ||
| * Saves a chat and its messages. If the chat exists, it updates it. | ||
| * This function should handle both creating new chats and appending messages. | ||
| * The PR implies complex logic for saving, including message IDs. | ||
| * This is a simplified version; PR #533 might have more granular message saving. | ||
| * This function handles both creating new chats and appending messages. | ||
| * | ||
| * All message IDs are guaranteed to be valid UUIDs. If a message has a | ||
| * duplicate ID within the same batch, a fresh UUID is generated for the | ||
| * duplicate — no suffix appending is used. | ||
| * | ||
| * @param chatData - The chat data to save. | ||
| * @param messagesData - An array of messages to save with the chat. | ||
| * @returns The saved chat ID. | ||
|
|
@@ -90,6 +126,13 @@ export async function saveChat(chatData: NewChat, messagesData: Omit<NewMessage, | |
| return db.transaction(async (tx: typeof db) => { | ||
| let chatId = chatData.id; | ||
|
|
||
| // Validate the chat ID if provided | ||
| if (chatId && !isValidUUID(chatId)) { | ||
| console.warn(`[chat-db] Invalid chat ID detected: "${chatId}". Chat may not have a valid UUID.`); | ||
| // Let the DB default handle it — do not override with a new ID, | ||
| // because the chat may already exist with that ID. | ||
| } | ||
|
|
||
| if (chatId) { // If chat ID is provided, assume update or append messages | ||
| const existingChat = await tx.select({ id: chats.id }).from(chats).where(eq(chats.id, chatId)).limit(1); | ||
| if (!existingChat.length) { | ||
|
|
@@ -108,26 +151,36 @@ export async function saveChat(chatData: NewChat, messagesData: Omit<NewMessage, | |
| } | ||
|
|
||
| if (!chatId) { | ||
| // console.error('Failed to establish chatId within transaction.'); // Optional: for server logs | ||
| throw new Error('Failed to establish chatId for chat operation.'); | ||
| } | ||
|
|
||
| // Save messages — deduplicate by ID first, then generate unique IDs for any remaining duplicates | ||
| // Save messages — normalize all IDs to valid UUIDs and deduplicate | ||
| // This prevents "ON CONFLICT DO UPDATE cannot affect row a second time" when the AI state | ||
| // contains multiple messages sharing the same ID (e.g., groupeId used for response + related + followup). | ||
| if (messagesData && messagesData.length > 0) { | ||
| const seenIds = new Set<string>(); | ||
| const messagesToInsert: typeof messages.$inferInsert[] = []; | ||
| const invalidIds: string[] = []; | ||
|
|
||
| for (const msg of messagesData) { | ||
| let id = msg.id ?? crypto.randomUUID(); | ||
| // Normalize the ID: strip invalid suffixes or generate a fresh UUID | ||
| const normalizedId = normalizeMessageId(msg.id); | ||
|
|
||
| // If we've already seen this ID in this batch, generate a unique one | ||
| while (seenIds.has(id)) { | ||
| id = `${id}-${Math.random().toString(36).substring(2, 8)}`; | ||
| // using generateUUID() — NOT suffix appending. | ||
| let id = normalizedId; | ||
| if (seenIds.has(id)) { | ||
| id = generateUUID(); | ||
| } | ||
| seenIds.add(id); | ||
|
|
||
| // Validate the final ID before insertion | ||
| if (!isValidUUID(id)) { | ||
| invalidIds.push(id); | ||
| console.error(`[chat-db] Generated ID is still invalid after normalization: "${id}". Skipping message.`); | ||
| continue; // Skip this message to avoid a DB error | ||
| } | ||
|
|
||
| messagesToInsert.push({ | ||
| ...msg, | ||
| id, | ||
|
|
@@ -136,7 +189,33 @@ export async function saveChat(chatData: NewChat, messagesData: Omit<NewMessage, | |
| }); | ||
| } | ||
|
|
||
| await tx.insert(messages).values(messagesToInsert).onConflictDoUpdate({ target: messages.id, set: { content: sql`EXCLUDED.content`, role: sql`EXCLUDED.role` } }); | ||
| // Log a summary if any IDs were problematic | ||
| if (invalidIds.length > 0) { | ||
| console.error(`[chat-db] Skipped ${invalidIds.length} message(s) with invalid UUIDs:`, invalidIds); | ||
| } | ||
|
|
||
| if (messagesToInsert.length > 0) { | ||
| try { | ||
| await tx.insert(messages).values(messagesToInsert).onConflictDoUpdate({ target: messages.id, set: { content: sql`EXCLUDED.content`, role: sql`EXCLUDED.role` } }); | ||
| } catch (dbError) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Catching the failed bulk statement inside this outer As a result, the new path cannot save the good messages when any bulk insert error occurs, despite the stated fallback behavior. Put each attempt in a savepoint (for example, |
||
| // If the bulk insert fails (e.g., due to a constraint violation), | ||
| // fall back to inserting messages one-by-one with individual error handling. | ||
| // Wrap each retry in a nested transaction (savepoint) so one failure doesn't | ||
| // abort the entire parent transaction. | ||
| console.error('[chat-db] Bulk insert failed, falling back to individual inserts:', dbError); | ||
|
|
||
| for (const msg of messagesToInsert) { | ||
| try { | ||
| await tx.transaction(async (nestedTx: any) => { | ||
| await nestedTx.insert(messages).values(msg).onConflictDoUpdate({ target: messages.id, set: { content: sql`EXCLUDED.content`, role: sql`EXCLUDED.role` } }); | ||
| }); | ||
| } catch (singleError) { | ||
| console.error(`[chat-db] Failed to save individual message (id: ${msg.id}):`, singleError); | ||
| // Continue processing remaining messages — don't break the loop | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
| return chatId; | ||
| }); | ||
|
|
@@ -155,6 +234,13 @@ export async function createMessage(messageData: NewMessage): Promise<Message | | |
| console.error('Missing required fields for creating a message.'); | ||
| return null; | ||
| } | ||
|
|
||
| // Ensure the message ID is a valid UUID | ||
| if (messageData.id && !isValidUUID(messageData.id)) { | ||
| console.warn(`[chat-db] Invalid message ID detected in createMessage: "${messageData.id}". Regenerating.`); | ||
| messageData = { ...messageData, id: generateUUID() }; | ||
| } | ||
|
|
||
| try { | ||
| const result = await db.insert(messages).values(messageData).returning(); | ||
| return result[0] || null; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.