-
Notifications
You must be signed in to change notification settings - Fork 59.3k
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
hotfix for x-goog-api-key
#5562
Conversation
The latest updates on your projects. Learn more about Vercel for Git βοΈ
|
WalkthroughThe changes in the Changes
Suggested reviewers
Poem
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 (2)
app/api/google.ts (2)
96-96
: LGTM! Consider handling multiple query parameters.The change correctly handles the
alt=sse
query parameter. However, consider if there might be cases where multiple query parameters need to be appended. If so, a more robust URL construction method might be beneficial.If multiple query parameters are possible, consider using the
URLSearchParams
API for more robust URL construction:const searchParams = new URLSearchParams(req.nextUrl.searchParams); if (searchParams.get("alt") === "sse") { searchParams.set("alt", "sse"); } const fetchUrl = `${baseUrl}${path}${searchParams.toString() ? `?${searchParams.toString()}` : ""}`;
104-106
: LGTM! Consider minor optimization.The changes are consistent with the earlier modifications and align with the PR objective. The use of the nullish coalescing operator is a good practice.
Consider extracting the API key retrieval logic into a separate function for better reusability and consistency:
function getApiKey(headers: Headers): string { return headers.get("x-goog-api-key") || (headers.get("Authorization") ?? "").replace("Bearer ", ""); } // Usage in fetchOptions "x-goog-api-key": getApiKey(req.headers),This would ensure consistent API key retrieval across the codebase and make future modifications easier.
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
π Files selected for processing (1)
- app/api/google.ts (2 hunks)
π Additional comments (2)
app/api/google.ts (2)
Line range hint
1-134
: Overall, the changes look good and address the PR objective.The modifications consistently prioritize the
x-goog-api-key
header and update the related logic throughout the file. The changes appear to be a suitable hotfix for the issue mentioned in the PR title.A few minor suggestions and verifications have been proposed to further improve the code:
- Verify the handling of empty API key fallback.
- Consider a more robust method for handling multiple query parameters.
- Extract the API key retrieval logic into a separate function for better reusability.
These suggestions are not critical but could enhance the code quality and maintainability.
26-27
: LGTM! Verify empty string fallback behavior.The change to prioritize the
x-goog-api-key
header aligns with the PR objective. However, ensure that using an empty string as a fallback doesn't lead to unexpected behavior in downstream code.Let's verify how the empty string fallback is handled:
β Verification successful
Empty string fallback is properly handled.
The implementation includes checks for empty
apiKey
inapp/api/google.ts
and other relevant files, ensuring that an empty string fallback does not lead to unexpected behavior in downstream code.π Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check how empty API key is handled in the codebase # Search for API key usage rg "apiKey" --type typescript -C 3Length of output: 69
Script:
#!/bin/bash # Description: Check how empty API key is handled in the codebase # Search for API key usage rg "apiKey" --type ts -C 3Length of output: 8298
Script:
#!/bin/bash # Description: Investigate how 'bearToken' and 'apiKey' are handled when empty strings are used # Search for 'bearToken' usage rg "bearToken" --type ts -C 3 # Search for 'apiKey' usage rg "apiKey" --type ts -C 3Length of output: 9432
Your build has completed! |
π» εζ΄η±»ε | Change Type
π εζ΄θ―΄ζ | Description of Change
π θ‘₯ε δΏ‘ζ― | Additional Information
Summary by CodeRabbit
New Features
Bug Fixes