fix(vite): make read git branch call async#2793
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 53 minutes and 38 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Vite base configuration file is updated to use asynchronous Git branch detection instead of synchronous calls. The Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
js/app/packages/app/vite.base.ts (1)
60-66:⚠️ Potential issue | 🟡 MinorWatcher callback still uses synchronous
readGitBranch().The stated goal of this PR is to prevent the dev server from hanging if reading the git branch hangs, but the chokidar
watcher.on('all', ...)handler at line 64 still calls the synchronousreadGitBranch(). A hanginggit rev-parsetriggered by a HEAD change would still block the Node event loop here. Consider switching this call site toreadGitBranchAsync()as well for consistency with the connection handler, and so the sync version can eventually be retired.♻️ Proposed change
- watcher.on('all', () => { - server.ws.send({ - type: 'custom', - event: 'git-branch:update', - data: readGitBranch(), - }); - }); + watcher.on('all', () => { + readGitBranchAsync().then((branch) => { + server.ws.send({ + type: 'custom', + event: 'git-branch:update', + data: branch, + }); + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/app/vite.base.ts` around lines 60 - 66, The watcher callback currently calls the synchronous readGitBranch(), which can block the event loop; change the watcher.on('all', ...) handler to call readGitBranchAsync() instead (e.g., make the callback async or use readGitBranchAsync().then(...).catch(...)) and pass the resolved branch data into server.ws.send({ type: 'custom', event: 'git-branch:update', data: ... }); ensure you handle rejections (log or send a safe fallback value) to avoid unhandled promise rejections and preserve non-blocking behavior used in the connection handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/vite.base.ts`:
- Around line 34-40: The Promise executor in readGitBranchAsync currently uses
the parameter name "resolve", which shadows the top-level "resolve" import from
node:path; rename the executor parameter (e.g. to "resolvePromise" or "res") to
avoid the shadowing and improve readability and update any corresponding usages
inside readGitBranchAsync (the exec callback's resolve call) to the new name so
the function behavior remains unchanged.
---
Outside diff comments:
In `@js/app/packages/app/vite.base.ts`:
- Around line 60-66: The watcher callback currently calls the synchronous
readGitBranch(), which can block the event loop; change the watcher.on('all',
...) handler to call readGitBranchAsync() instead (e.g., make the callback async
or use readGitBranchAsync().then(...).catch(...)) and pass the resolved branch
data into server.ws.send({ type: 'custom', event: 'git-branch:update', data: ...
}); ensure you handle rejections (log or send a safe fallback value) to avoid
unhandled promise rejections and preserve non-blocking behavior used in the
connection handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7e384eeb-ec46-4fb6-918e-855dbdebc83e
📒 Files selected for processing (1)
js/app/packages/app/vite.base.ts
So server doesn't hang if git branch read hangs