🌿 fix: Message Route Improvements (#3378)

* fix: do not throw errors for invalid convo id, simply return undefined

* feat: Add error handling and logging to message route definitions

* feat: Refactor Message.js to improve code organization and readability

* fix: Return undefined instead of throwing error for invalid conversation ID in Message.spec.js

api/models/Message.js

@@ -1,6 +1,6 @@
 const { z } = require('zod');
 const Message = require('./schema/messageSchema');
-const logger = require('~/config/winston');
+const { logger } = require('~/config');
 
 const idSchema = z.string().uuid();
 
@@ -27,42 +27,43 @@ const idSchema = z.string().uuid();
  * @param {string} [params.finish_reason] - Reason for finishing the message.
  * @param {number} [params.tokenCount] - The number of tokens in the message.
  * @param {string} [params.plugin] - Plugin associated with the message.
- * @param {Object[]} [params.plugins] - An array of plugins associated with the message.
+ * @param {string[]} [params.plugins] - An array of plugins associated with the message.
  * @param {string} [params.model] - The model used to generate the message.
  * @returns {Promise<TMessage>} The updated or newly inserted message document.
  * @throws {Error} If there is an error in saving the message.
  */
-async function saveMessage(
+async function saveMessage(req, params) {
-  req,
-  {
-    endpoint,
-    iconURL,
-    messageId,
-    newMessageId,
-    conversationId,
-    parentMessageId,
-    sender,
-    text,
-    isCreatedByUser,
-    error,
-    unfinished,
-    files,
-    isEdited,
-    finish_reason,
-    tokenCount,
-    plugin,
-    plugins,
-    model,
-  },
-) {
   try {
     if (!req || !req.user || !req.user.id) {
       throw new Error('User not authenticated');
     }
 
+    const {
+      text,
+      error,
+      model,
+      files,
+      plugin,
+      sender,
+      plugins,
+      iconURL,
+      endpoint,
+      isEdited,
+      messageId,
+      unfinished,
+      tokenCount,
+      newMessageId,
+      finish_reason,
+      conversationId,
+      parentMessageId,
+      isCreatedByUser,
+    } = params;
+
     const validConvoId = idSchema.safeParse(conversationId);
     if (!validConvoId.success) {
-      throw new Error('Invalid conversation ID');
+      logger.warn(`Invalid conversation ID: ${conversationId}`);
+      logger.info(params);
+      return;
     }
 
     const update = {

api/models/Message.spec.js

@@ -78,7 +78,7 @@ describe('Message Operations', () => {
 
     it('should throw an error for invalid conversation ID', async () => {
       mockMessage.conversationId = 'invalid-id';
-      await expect(saveMessage(mockReq, mockMessage)).rejects.toThrow('Invalid conversation ID');
+      await expect(saveMessage(mockReq, mockMessage)).resolves.toBeUndefined();
     });
   });
 

api/server/routes/messages.js

@@ -1,45 +1,75 @@
 const express = require('express');
-const router = express.Router();
 const { saveConvo, saveMessage, getMessages, updateMessage, deleteMessages } = require('~/models');
 const { requireJwtAuth, validateMessageReq } = require('~/server/middleware');
 const { countTokens } = require('~/server/utils');
+const { logger } = require('~/config');
 
+const router = express.Router();
 router.use(requireJwtAuth);
 
 /* Note: It's necessary to add `validateMessageReq` within route definition for correct params */
 router.get('/:conversationId', validateMessageReq, async (req, res) => {
-  const { conversationId } = req.params;
+  try {
-  res.status(200).send(await getMessages({ conversationId }, '-_id -__v -user'));
+    const { conversationId } = req.params;
+    const messages = await getMessages({ conversationId }, '-_id -__v -user');
+    res.status(200).json(messages);
+  } catch (error) {
+    logger.error('Error fetching messages:', error);
+    res.status(500).json({ error: 'Internal server error' });
+  }
 });
 
-// CREATE
 router.post('/:conversationId', validateMessageReq, async (req, res) => {
-  const message = req.body;
+  try {
-  const savedMessage = await saveMessage(req, { ...message, user: req.user.id });
+    const message = req.body;
-  await saveConvo(req.user.id, savedMessage);
+    const savedMessage = await saveMessage(req, { ...message, user: req.user.id });
-  res.status(201).send(savedMessage);
+    if (!savedMessage) {
+      return res.status(400).json({ error: 'Message not saved' });
+    }
+    await saveConvo(req.user.id, savedMessage);
+    res.status(201).json(savedMessage);
+  } catch (error) {
+    logger.error('Error saving message:', error);
+    res.status(500).json({ error: 'Internal server error' });
+  }
 });
 
-// READ
 router.get('/:conversationId/:messageId', validateMessageReq, async (req, res) => {
-  const { conversationId, messageId } = req.params;
+  try {
-  res.status(200).send(await getMessages({ conversationId, messageId }, '-_id -__v -user'));
+    const { conversationId, messageId } = req.params;
+    const message = await getMessages({ conversationId, messageId }, '-_id -__v -user');
+    if (!message) {
+      return res.status(404).json({ error: 'Message not found' });
+    }
+    res.status(200).json(message);
+  } catch (error) {
+    logger.error('Error fetching message:', error);
+    res.status(500).json({ error: 'Internal server error' });
+  }
 });
 
-// UPDATE
 router.put('/:conversationId/:messageId', validateMessageReq, async (req, res) => {
-  const { messageId, model } = req.params;
+  try {
-  const { text } = req.body;
+    const { messageId, model } = req.params;
-  const tokenCount = await countTokens(text, model);
+    const { text } = req.body;
-  const result = await updateMessage(req, { messageId, text, tokenCount });
+    const tokenCount = await countTokens(text, model);
-  res.status(201).json(result);
+    const result = await updateMessage(req, { messageId, text, tokenCount });
+    res.status(200).json(result);
+  } catch (error) {
+    logger.error('Error updating message:', error);
+    res.status(500).json({ error: 'Internal server error' });
+  }
 });
 
-// DELETE
 router.delete('/:conversationId/:messageId', validateMessageReq, async (req, res) => {
-  const { messageId } = req.params;
+  try {
-  await deleteMessages({ messageId });
+    const { messageId } = req.params;
-  res.status(204).send();
+    await deleteMessages({ messageId });
+    res.status(204).send();
+  } catch (error) {
+    logger.error('Error deleting message:', error);
+    res.status(500).json({ error: 'Internal server error' });
+  }
 });
 
 module.exports = router;