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

feat: optimize getHeaders #4934

Merged
merged 2 commits into from
Jul 6, 2024

Conversation

Dogtiti
Copy link
Member

@Dogtiti Dogtiti commented Jul 6, 2024

Summary by CodeRabbit

  • Refactor
    • Improved the logic for setting authentication headers to enhance code maintainability and readability.

@Dogtiti Dogtiti requested a review from lloydzhou July 6, 2024 03:16
Copy link

vercel bot commented Jul 6, 2024

@Dogtiti is attempting to deploy a commit to the NextChat Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

coderabbitai bot commented Jul 6, 2024

Walkthrough

The ClientApi class in app/client/api.ts has undergone a significant refactor to enhance the getHeaders function. The update introduces four helper functions—getConfig, getAuthHeader, getBearerToken, and validString—to simplify the process of setting authentication headers based on different provider types and access control configurations.

Changes

File Change Summary
app/client/api.ts Refactored getHeaders function to include getConfig, getAuthHeader, getBearerToken, and validString. Removed modelConfig and authHeader references.

Poem

In code's vast garden, changes bloom,
Headers refactored, clearer room.
Functions split and logic neat,
Authentication now complete.
With Bearer tokens in the air,
We streamlined flows with utmost care.
CodeRabbit hops with joy and cheer,
For cleaner code we hold dear! 🎉


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2d1f522 and 700b06f.

Files selected for processing (1)
  • app/client/api.ts (1 hunks)
Additional comments not posted (6)
app/client/api.ts (6)

160-164: Review the usage of useChatStore and useAccessStore.

Ensure that the state retrievals from useChatStore and useAccessStore are efficient and do not introduce any performance issues.


188-192: Review the getBearerToken function for correctness.

Ensure that the getBearerToken function correctly generates the bearer token and handles edge cases.

Verification successful

The getBearerToken function is correctly implemented and handles edge cases appropriately.

  • The function correctly checks if the apiKey is valid, trims it, and conditionally prefixes it with "Bearer ".
  • The usages of the function in the codebase are appropriate for setting authorization headers.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correctness of the `getBearerToken` function.

# Test: Search for the `getBearerToken` function and ensure it correctly generates the bearer token.
ast-grep --lang typescript --pattern $'function getBearerToken($_, $_) {
  $$$
}'

Length of output: 82


Script:

#!/bin/bash
# Locate the `getBearerToken` function definition in the TypeScript files.
rg 'function getBearerToken' --type ts

# Search for all usages of the `getBearerToken` function to verify how it's being used.
rg 'getBearerToken' --type ts

Length of output: 417


Script:

#!/bin/bash
# Extract the implementation of the `getBearerToken` function from `app/client/api.ts`.
sed -n '/function getBearerToken/,/}/p' app/client/api.ts

# Extract the lines where `getBearerToken` is used in `app/client/api.ts` for review.
rg 'getBearerToken' --context 5 app/client/api.ts

Length of output: 1067


194-196: Review the validString function for correctness.

Ensure that the validString function correctly validates the input string.

Verification successful

The validString function is correctly implemented.

The function correctly validates the input string by using optional chaining to handle null or undefined values and checking that the string length is greater than 0.

  • Location: app/client/api.ts, lines 194-196
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correctness of the `validString` function.

# Test: Search for the `validString` function and ensure it correctly validates the input string.
ast-grep --lang typescript --pattern $'function validString($_) {
  $$$
}'

Length of output: 75


Script:

#!/bin/bash
# Description: Locate the `validString` function in the codebase and review its implementation.

# Search for the `validString` function definition in the codebase.
rg 'function validString' --typescript

Length of output: 477


Script:

#!/bin/bash
# Description: Locate the `validString` function in the codebase and review its implementation.

# Search for the `validString` function definition in TypeScript files.
rg 'function validString' --type ts

Length of output: 100


Script:

#!/bin/bash
# Description: Extract and review the implementation of the `validString` function from `app/client/api.ts`.

# Extract the lines containing the `validString` function implementation.
sed -n '194,196p' app/client/api.ts

Length of output: 110


197-211: Ensure the refactored getHeaders function works correctly.

The refactored getHeaders function uses the new helper functions. Verify the overall correctness and efficiency of the refactored function.

Verification successful

The getHeaders function seems to be using the new helper functions correctly. To ensure the overall correctness and efficiency, we need to verify the implementations of the helper functions and check the usage of getHeaders throughout the codebase.


The refactored getHeaders function is correctly implemented and efficiently used throughout the codebase.

The helper functions (getConfig, getAuthHeader, getBearerToken, and validString) are correctly implemented, and the getHeaders function is widely used in various modules, indicating its proper integration.

  • app/store/access.ts
  • app/client/platforms/google.ts
  • app/client/platforms/openai.ts
  • app/client/platforms/anthropic.ts
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correctness and efficiency of the refactored `getHeaders` function.

# Test: Search for the `getHeaders` function and ensure it works correctly with the new helper functions.
ast-grep --lang typescript --pattern $'export function getHeaders() {
  $$$
}'

Length of output: 3394


Script:

#!/bin/bash
# Verify the implementation of the helper functions used in getHeaders

# Check the implementation of the getConfig function
ast-grep --lang typescript --pattern $'function getConfig() {
  $$$
}' ./app/client/api.ts

# Check the implementation of the getAuthHeader function
ast-grep --lang typescript --pattern $'function getAuthHeader(): string {
  $$$
}' ./app/client/api.ts

# Check the implementation of the getBearerToken function
ast-grep --lang typescript --pattern $'function getBearerToken(apiKey: string, noBearer: boolean = false): string {
  $$$
}' ./app/client/api.ts

# Check the implementation of the validString function
ast-grep --lang typescript --pattern $'function validString(x: string): boolean {
  $$$
}' ./app/client/api.ts

# Check the usage of the getHeaders function throughout the codebase
rg 'getHeaders' --type=ts --type=tsx

Length of output: 2189


Script:

#!/bin/bash
# Check the usage of the getHeaders function throughout the codebase
rg 'getHeaders' --type=ts

Length of output: 840


184-186: Review the getAuthHeader function for correctness.

Ensure that the getAuthHeader function correctly determines the authentication header based on the provider type.

Verification successful

The getAuthHeader function correctly determines the authentication header based on the provider type.

The function appropriately checks the providerName in modelConfig to set the authentication header for Azure and Anthropic providers.

  • The variable isAzure is correctly defined as modelConfig.providerName === ServiceProvider.Azure.
  • The variable isAnthropic is correctly defined as modelConfig.providerName === ServiceProvider.Anthropic.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correctness of the `getAuthHeader` function.

# Test: Search for the `getAuthHeader` function and ensure it correctly determines the authentication header.
ast-grep --lang typescript --pattern $'function getAuthHeader() {
  $$$
}'

Length of output: 75


Script:

#!/bin/bash
# Use ripgrep to search for the definition of the `getAuthHeader` function in the codebase.
rg 'function getAuthHeader' --type typescript

Length of output: 82


Script:

#!/bin/bash
# Use ripgrep to search for the definition of the `getAuthHeader` function in TypeScript files.
rg 'function getAuthHeader' --glob '*.ts'

Length of output: 98


Script:

#!/bin/bash
# Extract lines of code around the `getAuthHeader` function definition in the `app/client/api.ts` file.
rg -A 10 -B 10 'function getAuthHeader' app/client/api.ts

Length of output: 689


Script:

#!/bin/bash
# Search for the definitions of `isAzure` and `isAnthropic` in the `app/client/api.ts` file.
rg 'const isAzure' app/client/api.ts
rg 'const isAnthropic' app/client/api.ts

Length of output: 233


168-182: Ensure the logic in getConfig is correct and efficient.

The getConfig function consolidates various configurations. Verify the correctness of the logic and ensure it handles all edge cases.

Verification successful

The logic in getConfig is correct and efficient.

The getConfig function correctly consolidates various configurations, fetching the necessary API keys based on the provider and handling the enabled access control flag appropriately. No further edge cases or issues were identified in the code.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correctness and efficiency of the `getConfig` function.

# Test: Search for the `getConfig` function and ensure it handles all edge cases.
ast-grep --lang typescript --pattern $'function getConfig() {
  $$$
}'

Length of output: 1103


Script:

#!/bin/bash
# Search for the definition and usages of `chatStore` in the codebase.
ast-grep --lang typescript --pattern $'const chatStore = $_' 

# Search for the definition and usages of `accessStore` in the codebase.
ast-grep --lang typescript --pattern $'const accessStore = $_' 

# Search for the definition and usages of `currentSession` method in the codebase.
ast-grep --lang typescript --pattern $'currentSession($_) { $$$ }' 

# Search for the definition and usages of `mask` property in the codebase.
ast-grep --lang typescript --pattern $'mask: $_' 

# Search for the definition and usages of `modelConfig` property in the codebase.
ast-grep --lang typescript --pattern $'modelConfig: $_' 

Length of output: 1384


Script:

#!/bin/bash
# Correct the search pattern for the `currentSession` method.
ast-grep --lang typescript --pattern $'currentSession() { $$$ }' 

# Search for the definition and usages of `useChatStore` in the codebase.
ast-grep --lang typescript --pattern $'const useChatStore = $_' 

# Search for the definition and usages of `useAccessStore` in the codebase.
ast-grep --lang typescript --pattern $'const useAccessStore = $_' 

# Search for the definition and usages of `mask` property in the codebase.
ast-grep --lang typescript --pattern $'mask: $_' 

# Search for the definition and usages of `modelConfig` property in the codebase.
ast-grep --lang typescript --pattern $'modelConfig: $_' 

Length of output: 36615

@lloydzhou lloydzhou merged commit 7218f13 into ChatGPTNextWeb:main Jul 6, 2024
0 of 2 checks passed
@CoreJa
Copy link

CoreJa commented Jul 8, 2024

This PR is causing azure api 404. Please check it out. Rolling back to a previous commit solve the problem.

404 Error case
image

200 case
image

@lloydzhou lloydzhou self-assigned this Jul 8, 2024
@lloydzhou
Copy link
Contributor

lloydzhou commented Jul 8, 2024

@CoreJa

  1. 这个pr按道理应该不会引起类似的问题(这个pr只改了头信息的)
  2. 从(support azure deployment name #4930) 这个pr开始,只要是azure的api,node server中应该会走/api/azure/deployments/{deploy_id}/chat/completions这个请求。
  3. 所以,需要看一下是哪个地方配置导致azure的请求发送到了/api/openai/v1/chat/completions这边

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


@CoreJa

  1. This PR should not cause similar problems.
  2. Starting from (support azure deployment name #4930) this PR, as long as it is Azure’s API, the request /api/azure/deployments/{deploy_id}/chat/completions should be made in the node server.
  3. Therefore, you need to check which configuration causes azure requests to be sent to /api/openai/v1/chat/completions

@CoreJa
Copy link

CoreJa commented Jul 8, 2024

@lloydzhou 你说得对,vercel的deploy好像是按天来的,所以我只看到了当前版本#4934和上一版本#4850

我的配置内容如下:

image

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


@lloydzhou You are right, vercel's deployment seems to be on a daily basis, so I only saw the current version #4934 and the previous version #4850.

My configuration content is as follows:

image

@CoreJa
Copy link

CoreJa commented Jul 8, 2024

@lloydzhou hi, 我定位了问题based on #4930 (comment)

我理解应该是从#4930后,如果custom_model指定gpt-4o的话,会出现俩option(azure/openai),同时它默认的选择是openai。我认为这个做法没道理,特别是在环境变量已经指定了azure_url的情况下。

我会在#4930下重述一遍。这个pr可以不用回复了。

@lloydzhou lloydzhou deleted the feature/client-headers branch August 1, 2024 05:14
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.

4 participants