Skip to content
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

Fix/isValidMessage #5579

Closed
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion app/store/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,7 @@ export const useChatStore = createPersistStore(
},
onFinish(message) {
console.log("[Memory] ", message);
if (!isValidMessage(message)) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Potential unvalidated message processing detected in app/store/chat.ts at line 696.

Please ensure that isValidMessage is invoked consistently wherever messages are processed to maintain validation integrity.

🔗 Analysis chain

Verify that message validation is consistently applied

In the onFinish callback, the check if (!isValidMessage(message)) return; ensures that invalid messages are not processed. To maintain consistency and robustness, ensure that all instances where messages are processed include validation using isValidMessage.

Run the following script to check for other message handling without validation:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find instances where messages are handled without validation.
# Expected: All message processing should involve validation via isValidMessage.

rg --type ts --no-heading --line-number --color never 'function\s+\w+\(.*message.*\)' app/store/ | while read -r line; do
  file=$(echo "$line" | cut -d: -f1)
  lineno=$(echo "$line" | cut -d: -f2)
  function_def=$(echo "$line" | cut -d: -f3-)
  function_body=$(awk "NR>$lineno && NR<($lineno+20)" "$file")
  if ! echo "$function_body" | grep -q "isValidMessage"; then
    echo "Potential unvalidated message in $file:$lineno - $function_def"
  fi
done

Length of output: 2295

get().updateCurrentSession((session) => {
session.lastSummarizeIndex = lastSummarizeIndex;
session.memoryPrompt = message; // Update the memory prompt for stored it in local storage
Expand All @@ -735,7 +736,26 @@ export const useChatStore = createPersistStore(
}

function isValidMessage(message: any): boolean {
return typeof message === "string" && !message.startsWith("```json");
if (typeof message !== "string") {
return false;
}
if (message.startsWith("```") && message.endsWith("```")) {
const codeBlockContent = message.slice(3, -3).trim();
const jsonString = codeBlockContent.replace(/^json\s*/i, '').trim();
try {
// 返回 json 格式消息,error 字段为 true 或者包含 error.message 字段,判定为错误回复,否则为正常回复
const jsonObject = JSON.parse(jsonString);
if (jsonObject?.error == true || jsonObject?.error?.message) {
return false;
}
return true;
} catch (e) {
console.log("Invalid JSON format.");
// 非 json 格式,通常可认为是正常回复
return true;
}
}
return true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance robustness of isValidMessage function

The isValidMessage function is a good addition to validate messages. However, consider the following improvements:

  1. Handle different types of code blocks, not just JSON.
  2. Improve error handling to be more specific about what went wrong.
  3. Consider additional validation for non-JSON messages.

Here's a suggested refactor:

 function isValidMessage(message: any): boolean {
   if (typeof message !== "string") {
     return false;
   }
   if (message.startsWith("```") && message.endsWith("```")) {
     const codeBlockContent = message.slice(3, -3).trim();
-    const jsonString = codeBlockContent.replace(/^json\s*/i, '').trim();
+    if (codeBlockContent.toLowerCase().startsWith('json')) {
+      const jsonString = codeBlockContent.replace(/^json\s*/i, '').trim();
+      try {
+        const jsonObject = JSON.parse(jsonString);
+        return !jsonObject?.error?.message;
+      } catch (e) {
+        console.log("Invalid JSON format:", e.message);
+        return false;
+      }
+    }
+    // Non-JSON code blocks are considered valid
+    return true;
   }
-    try {
-      // 返回 json 格式消息,error 字段为 true 或者包含 error.message 字段,判定为错误回复,否则为正常回复
-      const jsonObject = JSON.parse(jsonString);
-      if (jsonObject?.error == true || jsonObject?.error?.message) {
-        return false;
-      }
-      return true;
-    } catch (e) {
-      console.log("Invalid JSON format.");
-      // 非 json 格式,通常可认为是正常回复
-      return true;
-    }
-  }
   return true;
 }

This refactor:

  • Only attempts to parse JSON if the code block explicitly starts with "json".
  • Treats non-JSON code blocks as valid messages.
  • Improves error logging for invalid JSON.
  • Simplifies the overall logic.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (typeof message !== "string") {
return false;
}
if (message.startsWith("```") && message.endsWith("```")) {
const codeBlockContent = message.slice(3, -3).trim();
const jsonString = codeBlockContent.replace(/^json\s*/i, '').trim();
try {
// 返回 json 格式消息,error 字段为 true 或者包含 error.message 字段,判定为错误回复,否则为正常回复
const jsonObject = JSON.parse(jsonString);
if (jsonObject?.error == true || jsonObject?.error?.message) {
return false;
}
return true;
} catch (e) {
console.log("Invalid JSON format.");
// 非 json 格式,通常可认为是正常回复
return true;
}
}
return true;
function isValidMessage(message: any): boolean {
if (typeof message !== "string") {
return false;
}
if (message.startsWith("```") && message.endsWith("```")) {
const codeBlockContent = message.slice(3, -3).trim();
if (codeBlockContent.toLowerCase().startsWith('json')) {
const jsonString = codeBlockContent.replace(/^json\s*/i, '').trim();
try {
const jsonObject = JSON.parse(jsonString);
return !jsonObject?.error?.message;
} catch (e) {
console.log("Invalid JSON format:", e.message);
return false;
}
}
// Non-JSON code blocks are considered valid
return true;
}
return true;
}

}
},

Expand Down
96 changes: 96 additions & 0 deletions test/isValidMessage.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
function isValidMessage(message: any): boolean {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import { isValidMessage } from @/app/store/chat

if (typeof message !== "string") {
return false;
}
if (message.startsWith("```") && message.endsWith("```")) {
const codeBlockContent = message.slice(3, -3).trim();
const jsonString = codeBlockContent.replace(/^json\s*/i, '').trim();
try {
// 返回 json 格式消息,含 error.message 字段,判定为错误回复,否则为正常回复
const jsonObject = JSON.parse(jsonString);
if (jsonObject?.error == true || jsonObject?.error?.message) {
return false;
}
return true;
} catch (e) {
console.log("Invalid JSON format.");
// 非 json 格式,大概率是正常回复
return true;
}
}
return true;
}

describe("is valid message module", () => {
test("error msg no.0", () => {
const message = "Hello! How can I assist you today?";
expect(isValidMessage(message)).toBe(true);
});
test("error msg no.1", () => {
const message = `
\`\`\`json
{
"error": true,
"msg": "金额不足"
}
\`\`\`
`;
expect(isValidMessage(message)).toBe(false);
});
test("error msg no.2", () => {
const message = `
\`\`\`
{
"error": {
"message": "You didn't provide an API key. You need to provide your API key in an Authorization header using Bearer auth (i.e. Authorization: Bearer YOUR_KEY), or as the password field (with blank username) if you're accessing the API from your browser and are prompted for a username and password. You can obtain an API key from https://platform.openai.com/account/api-keys.",
"type": "invalid_request_error",
"param": null,
"code": null
}
}
\`\`\`
`;
expect(isValidMessage(message)).toBe(false);
});
test("error msg no.3", () => {
const message = `
\`\`\`
{
"error": {
"message": "Incorrect API key provided: 123456. You can find your API key at https://platform.openai.com/account/api-keys.",
"type": "invalid_request_error",
"param": null,
"code": "invalid_api_key"
}
}
\`\`\`
`;
expect(isValidMessage(message)).toBe(false);
});
test("error msg no.4", () => {
const message = `
\`\`\`
{
"error": {
"message": "当前分组 default 下对于模型 gpt-4 无可用渠道 (request id: 2024101214105418395279367750613)",
"type": "one_api_error"
}
}
\`\`\`
`;
expect(isValidMessage(message)).toBe(false);
});
test("error msg no.5", () => {
const message = `
\`\`\`
{
"error": {
"message": "该令牌状态不可用 (request id: 2024101214105418395279367750613)",
"type": "one_api_error"
}
}
\`\`\`
`;
expect(isValidMessage(message)).toBe(false);
});
});
Loading