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

Fixed BingChat to recent API changes. #41

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

gunpal5
Copy link

@gunpal5 gunpal5 commented Oct 6, 2023

Fixed BingChat to recent API changes. Now it is absolutely necessary to provide all of the Bing Cookies.
Added codes to ensure the conversation is authenticated.
Added retry mechanism on WebSocketException

this PR provides fixes to issues #40 #39 #38 #36

To use command line tool:

  1. Copy Bing Cookies using Cookies Editor extension and save to a file.
  2. Set BING_COOKIES_FILE environment variable by using this command
    set [BING_COOKIES_FILE=[<path to cookies file>]]
  3. dotnet bingchat

Gunpal Jain added 2 commits October 7, 2023 04:37
… to provide all the Bing Cookies.

Added codes to ensure the conversation is authenticated.
Added retry mechanism on WebSocketException
@gunpal5 gunpal5 changed the title Fixed BingChat to recent APIs changes. Fixed BingChat to recent API changes. Oct 6, 2023
@gunpal5 gunpal5 mentioned this pull request Oct 7, 2023
@neon-sunset
Copy link
Collaborator

neon-sunset commented Oct 7, 2023

Thank you for the pull request @gunpal5. However, it cannot be merged as is. There are a few points to address first:

  • Remove irrelevant comments
  • Use JsonSerializerContext instead of reflection-based serialization (<T>) (otherwise this will break AOT)
  • Double-check if the change to parse the cookie file is necessary - can we keep/describe all the necessary arguments via BingChatClientOptions type, where loading/parsing full cookie container remains an option for special use cases?
  • Fix syntax formatting errors
  • Remove transient build/IDE files

@neon-sunset neon-sunset self-requested a review October 7, 2023 16:18
Used JsonSerializerContext instead of reflection-based serialization
Removed transient IDE files
@gunpal5
Copy link
Author

gunpal5 commented Oct 7, 2023

@neon-sunset I made necessary changes you asked.

Regarding cookie files, I am not sure which cookie Bing is using to authenticate the chat. only passing _U and/or KievRPSSecAuth are not enough. someone else might figure out the right cookies to pass as arguments in BingChatClientOptions

I would advise to add a new argument CookieContainer in BIngChatClientOptions to pass the Bing Cookies, that would be more appropriate than to pass Cookies Editor file path.

@gunpal5
Copy link
Author

gunpal5 commented Nov 7, 2023

@neon-sunset It's been like a month, people need it. Please review and merge

@neon-sunset
Copy link
Collaborator

I understand that you would like to have this PR merged.
But I can't do it because it still has issues and does not approach the problem in a way this library is designed - it reverse engineers original BingChat logic and requires to configure only what's actually necessary. Forcing the users to dump all of the cookies being the only way for it to work is a workaround. My suggestion would be inspecting the API interaction flow closer and also checking out what the libraries in other languages do, and then porting their fixes to this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This file does not belong to the PR

@@ -107,11 +115,15 @@ public async Task<BingChatConversation> CreateConversation()
throw new BingChatException(message);
}

var encryptedConversationSignature = rawResponse.Headers.GetValues("X-Sydney-EncryptedConversationSignature").FirstOrDefault();
Copy link
Collaborator

@neon-sunset neon-sunset Nov 7, 2023

Choose a reason for hiding this comment

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

If this doesn't work without encryptedConversationSignature, shouldn't it be First() instead of FirstOrDefault()?

{
_request = new BingChatRequest(clientId, conversationId, conversationSignature, tone);
_encryptedConversationSignature = encryptedConversationSignature;
this._cookies = cookies;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
this._cookies = cookies;
_cookies = cookies;

HubProtocol,
HubEndpoint,
new UriEndPoint(new Uri(uri)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to cache this property instead of creating it on each request.

return new BingChatClient(new BingChatClientOptions
{
CookieU = string.IsNullOrWhiteSpace(cookie) ? null : cookie,
CookieFilePath = string.IsNullOrEmpty(cookieFile)? null: cookieFile,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
CookieFilePath = string.IsNullOrEmpty(cookieFile)? null: cookieFile,
CookieFilePath = string.IsNullOrEmpty(cookieFile) ? null : cookieFile,

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert all changes in this file.

@neon-sunset
Copy link
Collaborator

neon-sunset commented Nov 7, 2023

I'll need to give it another look over some time later with a proper IDE.

}
catch (WebSocketException)
{
Thread.Sleep(100);
Copy link
Collaborator

@neon-sunset neon-sunset Nov 7, 2023

Choose a reason for hiding this comment

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

You must never use Thread.Sleep(...) in async methods or methods that can be called from async. The correct way to do it is to use Task.Delay(...) instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants