feat: reduce vercel function duration#226
Conversation
📝 WalkthroughWalkthroughAdded Cache-Control headers to several SVG API endpoints, limited GitHub token rotation to only on 401/403, reduced GraphQL retry attempts, and updated test/CI configs, Node version, and Jest/Babel tooling; plus minor test and utility adjustments. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @api/cards/most-commit-language.ts:
- Around line 37-46: The code mutates process.env.GITHUB_TOKEN via
changToNextGitHubToken() causing cross-request race conditions; instead keep
token state local to the request by removing calls that change process.env and
use a request-scoped variable (e.g., localToken or tokenIndex) inside the
handler and any retry loop; update calls to getNextGitHubToken(tokenIndex) or an
equivalent helper to return the next token/value and assign it to the local
variable, and pass that local token into the functions that call GitHub so all
retries use the request-local value without touching process.env.GITHUB_TOKEN.
🧹 Nitpick comments (3)
api/cards/repos-per-language.ts (1)
22-26: Consider simplifying the exclude array processing.The current approach uses
forEachto mutate an array. Consider usingmapandfilterfor a more functional style.♻️ Proposed refactor
- let excludeArr = <string[]>[]; - exclude.split(',').forEach(function (val) { - const translatedLanguage = translateLanguage(val); - excludeArr.push(translatedLanguage.toLowerCase()); - }); + const excludeArr = exclude + .split(',') + .map(val => translateLanguage(val).toLowerCase()) + .filter(val => val.length > 0);api/cards/productive-time.ts (1)
20-43: Consider extracting the retry-with-token-rotation pattern into a shared utility.All five card endpoints (repos-per-language, profile-details, stats, productive-time, most-commit-language) now share an identical retry loop pattern with token rotation. Consider refactoring this into a shared higher-order function or middleware to reduce duplication and improve maintainability.
Example structure:
async function withTokenRotation<T>( fn: () => Promise<T>, onError?: (err: any) => void ): Promise<T>api/cards/most-commit-language.ts (1)
8-26: Trim/filterexcludevalues to avoid translating/pushing empty strings (and simplify typing).
Right nowexclude = ''producesexcludeArr = [''](after translation), which is at best noise and at worst a subtle behavior change.Proposed fix
- let excludeArr = <string[]>[]; - exclude.split(',').forEach(function (val) { - const translatedLanguage = translateLanguage(val); - excludeArr.push(translatedLanguage.toLowerCase()); - }); + const excludeArr: string[] = exclude + .split(',') + .map((val) => translateLanguage(val.trim()).toLowerCase()) + .filter((val) => val.length > 0);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
api/cards/most-commit-language.tsapi/cards/productive-time.tsapi/cards/profile-details.tsapi/cards/repos-per-language.tsapi/cards/stats.tssrc/utils/request.ts
🧰 Additional context used
🧬 Code graph analysis (5)
api/cards/most-commit-language.ts (4)
api/cards/profile-details.ts (1)
req(6-40)api/cards/repos-per-language.ts (1)
req(7-51)api/cards/stats.ts (1)
req(6-40)api/utils/github-token-updater.ts (1)
changToNextGitHubToken(1-8)
api/cards/stats.ts (2)
api/cards/profile-details.ts (1)
req(6-40)api/utils/github-token-updater.ts (1)
changToNextGitHubToken(1-8)
api/cards/repos-per-language.ts (5)
api/cards/most-commit-language.ts (1)
req(7-52)api/cards/productive-time.ts (1)
req(6-44)api/cards/profile-details.ts (1)
req(6-40)api/cards/stats.ts (1)
req(6-40)api/utils/github-token-updater.ts (1)
changToNextGitHubToken(1-8)
api/cards/productive-time.ts (5)
api/cards/most-commit-language.ts (1)
req(7-52)api/cards/profile-details.ts (1)
req(6-40)api/cards/repos-per-language.ts (1)
req(7-51)api/cards/stats.ts (1)
req(6-40)api/utils/github-token-updater.ts (1)
changToNextGitHubToken(1-8)
api/cards/profile-details.ts (5)
api/cards/most-commit-language.ts (1)
req(7-52)api/cards/productive-time.ts (1)
req(6-44)api/cards/repos-per-language.ts (1)
req(7-51)api/cards/stats.ts (1)
req(6-40)api/utils/github-token-updater.ts (1)
changToNextGitHubToken(1-8)
🔇 Additional comments (6)
api/cards/repos-per-language.ts (1)
34-34: LGTM! Cache-Control header and refined error handling improve performance.The Cache-Control header enables client and CDN caching (4h client, 24h CDN), reducing redundant requests. The refined error handling correctly limits token rotation to authentication errors (401/403), avoiding unnecessary retries for other error types.
Also applies to: 40-45
src/utils/request.ts (1)
14-14: Significant retry reduction - monitor impact on reliability.Reducing retry attempts from 10 to 3 is a substantial change that will surface failures faster and reduce function duration. However, this could impact reliability if the GitHub API experiences temporary issues.
Consider monitoring error rates after deployment to ensure the reduced retry count doesn't negatively affect user experience, especially given that the card endpoints now also limit token rotation to only 401/403 errors.
api/cards/profile-details.ts (1)
22-22: LGTM! Consistent caching and error handling improvements.The changes mirror the pattern applied across all card endpoints: Cache-Control header for performance and refined token rotation logic limited to authentication errors.
Also applies to: 28-33
api/cards/stats.ts (1)
22-22: LGTM! Consistent implementation across card endpoints.The Cache-Control header and refined error handling follow the same pattern as other card endpoints, ensuring consistency across the API.
Also applies to: 28-33
api/cards/productive-time.ts (1)
26-26: LGTM! Consistent caching and error handling.The changes align with the pattern applied across all card endpoints.
Also applies to: 32-37
api/cards/most-commit-language.ts (1)
32-36: Cache-Control header addition looks good for reducing repeat work.
Header is set only on successful SVG responses, which also avoids caching error cards by accident.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @jest.config.js:
- Line 14: The collectCoverageFrom pattern in jest.config.js currently uses
'**/*.{ts,jx}' which likely contains a typo ('jx'); update the pattern used by
collectCoverageFrom to include the correct extensions you want (e.g., replace
'jx' with 'jsx' or add both 'jsx' and 'tsx' as needed), so the pattern becomes
something like including '**/*.{ts,tsx,jsx}' or the exact set of extensions your
project uses to ensure coverage is collected for the intended files.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.github/workflows/test-and-lint.yml.node-versionapi/cards/most-commit-language.tsapi/cards/productive-time.tsapi/cards/profile-details.tsapi/cards/repos-per-language.tsapi/cards/stats.tsbabel.config.jsjest.config.jsjest.config.jsonpackage.jsontests/github-api/commits-per-language.test.tstests/github-api/repos-per-language.test.tstests/utils/file-writer.test.ts
💤 Files with no reviewable changes (1)
- jest.config.json
✅ Files skipped from review due to trivial changes (3)
- tests/github-api/commits-per-language.test.ts
- .node-version
- .github/workflows/test-and-lint.yml
🚧 Files skipped from review as they are similar to previous changes (4)
- api/cards/productive-time.ts
- api/cards/repos-per-language.ts
- api/cards/most-commit-language.ts
- api/cards/stats.ts
🧰 Additional context used
🧬 Code graph analysis (3)
api/cards/profile-details.ts (1)
api/utils/github-token-updater.ts (1)
changToNextGitHubToken(1-8)
tests/utils/file-writer.test.ts (1)
src/utils/file-writer.ts (1)
OUTPUT_PATH(5-5)
tests/github-api/repos-per-language.test.ts (1)
src/github-api/repos-per-language.ts (1)
getRepoLanguages(64-95)
🔇 Additional comments (8)
api/cards/profile-details.ts (2)
22-22: LGTM! Cache-Control header improves performance.The added Cache-Control header will significantly reduce redundant requests by caching SVG responses for 4 hours client-side and 24 hours on the CDN. This directly supports the PR's goal of reducing Vercel function duration.
28-33: LGTM! Error handling refinement prevents wasteful retries.The refined logic now only rotates GitHub tokens and retries on authentication-related errors (401/403), which include rate limiting and auth failures. Non-auth errors are immediately rethrown, preventing unnecessary token cycling. This tightening aligns with the PR's goal of reducing function duration by limiting unnecessary retries.
tests/github-api/repos-per-language.test.ts (1)
136-137: LGTM!The test correctly validates case-insensitive language exclusion. The mock returns languages with proper casing (Rust, Jupyter Notebook), the exclusion list uses lowercase ('rust', 'jupyter notebook'), and the expected results correctly exclude the matching languages while preserving Kotlin and Java.
babel.config.js (1)
1-3: LGTM!Standard Babel configuration for transpiling tests. The
targets: { node: 'current' }setting correctly aligns with the Node 22.x engine requirement.tests/utils/file-writer.test.ts (1)
2-2: Good modernization for Node 22 compatibility.Replaced deprecated
rmdirSyncwithrmSync, which is the recommended API for Node.js 14.14.0+. This change aligns with the Node 22.x upgrade.Also applies to: 6-6
package.json (3)
35-36: Node 22.x upgrade and tooling updates look good.The dependency updates align well with the Node.js 22.x engine upgrade:
- Added Babel tooling for test transpilation
- Updated @types/node to v25 for Node 22 compatibility
- Updated build tools (@vercel/ncc)
These changes support the infrastructure modernization.
Also applies to: 40-40, 42-42, 45-45, 62-62
52-52: Clarify the pre-commit version change.The codebase currently specifies
"pre-commit": "^1.0.10"with an actual installed version of1.2.2(per package-lock.json). The commit message indicates this was a "bump packages version," suggesting an upgrade rather than a downgrade. Git history does not show a previous version to confirm the claimed downgrade from^1.2.2to^1.0.10. If a downgrade did occur, document the reason; otherwise, clarify the actual version change intent.Likely an incorrect or invalid review comment.
43-43: @vercel/node v2.x compatibility is verified and appropriate for this codebase.The upgrade to v2.3.0 is compatible. The codebase uses standard VercelRequest and VercelResponse types (stable across v1 and v2) and specifies Node 22.x in engines, which is compatible with @vercel/node v2.x. The vercel.json configuration uses the modern format without deprecated runtime declarations. No API-level breaking changes affect the current implementation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @package.json:
- Around line 51-54: Update the ts-jest dependency to a v30 release in
package.json (replace "ts-jest": "^29.4.6" with a compatible "^30.x" range) to
match Jest v30; then verify and, if needed, bump @types/jest and babel-jest to
versions compatible with Jest 30 to avoid peer dependency conflicts (ensure
package.json entries for "ts-jest", "@types/jest", and "babel-jest" are
aligned), run npm/yarn install and the test suite to confirm compatibility.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json
🔇 Additional comments (4)
package.json (4)
35-36: > Likely an incorrect or invalid review comment.
52-52: Verify pre-commit version intent.The package.json shows "pre-commit": "^1.0.10", but I cannot confirm from the available git history whether this represents a downgrade or intentional change from a previous version. Clarify whether version 1.0.10 is correct for your requirements, as this semantic version is lower than 1.2.2 (if applicable).
62-62: No action needed. Node 22 compatibility is already in place:
- Vercel support: Node 22 is fully supported for serverless functions (available since Nov 22, 2024)
- Dependencies: All dependencies are compatible with Node 22; no native modules present
- Consistency: Node version is consistent across package.json (22.x), .node-version (22.13.0), and GitHub Actions workflows
- No breaking changes: The TypeScript configuration and all dependencies support Node 22 without issues
42-43: No action needed. The codebase is already successfully using @vercel/node v2.3.0 across multiple API endpoints (theme.ts, profile-details.ts, stats.ts, most-commit-language.ts, productive-time.ts, repos-per-language.ts) with the standard VercelRequest and VercelResponse types, indicating full compatibility.Likely an incorrect or invalid review comment.
| "jest": "^30.2.0", | ||
| "pre-commit": "^1.0.10", | ||
| "prettier": "2.2.1", | ||
| "ts-jest": "^27.0.7", | ||
| "ts-jest": "^29.4.6", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ts-jest compatibility matrix Jest 30 support
💡 Result:
Short answer: As of the current docs/releases, ts-jest does not yet publish an official release that declares support for Jest 30 — ts-jest docs and releases are on the 29.x line while Jest 30 was released separately. See ts-jest version-checking/docs (29.x) and the ts-jest release list, and the Jest 30 announcement. [1][2][3]
Sources
- ts-jest version checking / docs (29.x). [1]
- ts-jest release history showing 29.x releases. [2]
- Jest 30 release / migration guide. [3]
If you want, I can check the ts-jest GitHub/npm package.json for any recent 30-supporting release and report exact peerDependencies.
🌐 Web query:
ts-jest v30 Jest 30 peer dependencies npm
💡 Result:
Short answer: ts-jest’s major version follows Jest’s major version — ts-jest v30 expects Jest 30 as a peer dependency. Install matching versions (and TypeScript/@types/jest) with npm, e.g.:
npm install --save-dev jest@^30 ts-jest@^30 typescript @types/jest. [1][2]
Notes:
- If npm reports ERESOLVE peer-conflict errors, either install matching versions for other packages that declare Jest peers, or use npm flags as a last resort (e.g. --legacy-peer-deps or --force) — but matching peer versions is the correct fix. [3][1]
Sources:
- ts-jest package/docs (peer/version policy & install). [1]
- Jest upgrade / Jest 30 compatibility notes. [2]
- Example issues showing peer-dependency conflicts and ERESOLVE behavior (discussion). [3]
Update ts-jest to v30 for Jest 30 compatibility.
ts-jest v29 does not officially support Jest v30. ts-jest v30 is required as a peer dependency for Jest v30. Update ts-jest from ^29.4.6 to ^30 and verify that @types/jest and babel-jest (currently at v29) are also compatible with Jest v30, as peer dependency conflicts may arise.
🤖 Prompt for AI Agents
In @package.json around lines 51 - 54, Update the ts-jest dependency to a v30
release in package.json (replace "ts-jest": "^29.4.6" with a compatible "^30.x"
range) to match Jest v30; then verify and, if needed, bump @types/jest and
babel-jest to versions compatible with Jest 30 to avoid peer dependency
conflicts (ensure package.json entries for "ts-jest", "@types/jest", and
"babel-jest" are aligned), run npm/yarn install and the test suite to confirm
compatibility.
1 participant
This pull request focuses on improving HTTP response handling and error management for several API card endpoints, as well as adjusting retry logic in the request utility. The main changes include setting cache control headers for better caching of SVG card responses, refining error handling to only retry on specific authentication errors, and reducing the default retry count for outgoing requests.
API Card Endpoints Improvements:
Cache-Controlheader (public, max-age=14400, s-maxage=86400) on all SVG card responses to enable better caching and reduce redundant requests. [1] [2] [3] [4] [5]Request Utility Adjustment:
src/utils/request.tsfrom 10 to 3, making request failures surface faster and reducing unnecessary retries.Summary by CodeRabbit
Performance Improvements
Bug Fixes
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.