-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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: refresh context for UnauthorizedRequest from Bing #658
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Perhaps, a relogin is required? |
For the
The crucial implementation step involves adding a retry flag to the context and utilizing it for both retry attempts and error display. This flag serves the additional purpose of preventing an infinite recursion loop in the chat. If you agree with this approach, I will proceed with the implementation. |
Thank you @samanhappy for the detailed deep thinking. But IMO, the solution is too smart. I prefer showing the error message and let the user determine what to do next. Just as Bing Chat's behavior. |
When using Bing Chat, user won't get an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @samanhappy,
Hope you're doing well! I wanted to check if the token issue is still something you're interested in tackling. It seems like it might still need some attention. Let me know what you think!
Hi @PeterDaveHello , what is your opinion about this issue? |
@samanhappy I'm not sure if creating a new conversation is needed, but if it is, then we can try your solution. |
WalkthroughThe changes in this pull request enhance the error handling within the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/bots/microsoft/BingChatBot.js (1)
184-187
: LGTM! Consider adding retry limits and logging.The changes effectively address the issue of handling both "InvalidSession" and "UnauthorizedRequest" errors by creating a new conversation context and retrying the prompt. This aligns well with the PR objectives and the proposed solution discussed in the comments.
Consider the following improvements:
- Implement a retry limit to prevent potential infinite loops:
const MAX_RETRIES = 3; let retryCount = 0; // Inside the if block: if (retryCount < MAX_RETRIES) { retryCount++; // Create a new conversation and retry // ... } else { reject(new Error(i18n.global.t("bot.maxRetriesReached"))); }
- Add logging for the specific error type:
console.log(`Handling ${event.item.result.value} error. Retrying...`);
- Consider adding a small delay before retrying to avoid potential rate-limiting:
await new Promise(resolve => setTimeout(resolve, 1000)); // 1 second delayThese suggestions can enhance the robustness and debuggability of the error handling mechanism.
Resolve #629
When token is expired, Bing will return such message
{"type":2,"invocationId":"1","item":{"firstNewMessageIndex":null,"defaultChatName":null,"result":{"value":"UnauthorizedRequest","message":"Token issued by https://sydney.bing.com/sydney is invalid (IDX10223: Lifetime validation failed. The token is expired. ValidTo (UTC): '12/8/2023 3:48:23 AM', Current time (UTC): '12/8/2023 3:58:42 AM'.)","error":"UnauthorizedRequest","renewCert":true,"serviceVersion":"20231207.96"}}}
There should be other scenarios for
UnauthorizedRequest
, I'm not sure whether it's appropriate to use it for direct comparison, what is your opinion ? @sunnerSummary by CodeRabbit