Draft
Conversation
gaureshpai
requested changes
Oct 3, 2025
Owner
gaureshpai
left a comment
gaureshpai
left a comment
There was a problem hiding this comment.
- Do consider staring the repository
- Can you please provide effective screenshots and screen recording describing the changes
Thank you for your contributions
Author
A retry fn has been added to retry any failure a given number of times (say a package err) and a proper error handling has been added to check for the mechanism (test-error-handling.js) Ex- If node and npm installed versions are lower than the one mentioned inside utils.js, an error will be thrown at the end inside the terminal (errors.push(err.message)) 
|
Owner
|
Hey @saatvik-10 can u please provide screen recording describing the changes, run npm pack, and execute the tgz file for the error check, Thank you |
Owner
|
Any updates @saatvik-10 ? |
Owner
|
Any updates on this PR @saatvik-10 ? |
gaureshpai
removed
question
Owner
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis pull request introduces error handling and compatibility validation to the create-next-quick CLI. New features include Node.js version enforcement, extended package manager detection (npm, yarn, pnpm), retry logic for dependency installation, compatibility warnings, and conditional setup pathways for Shadcn UI and ORM frameworks. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as CLI Process
participant Validator as Environment<br/>Validator
participant PkgMgr as Package Manager<br/>Detector
participant Compat as Compatibility<br/>Checker
participant Installer as Installer with<br/>Retry Logic
participant Setup as Conditional Setup<br/>(Shadcn/ORM)
participant FileSystem as File System
User->>CLI: Start create-next-quick
CLI->>Validator: validateEnvironment()
Validator-->>CLI: Node version check, npm presence
CLI->>PkgMgr: Detect npm/yarn/pnpm availability
PkgMgr-->>CLI: Available package managers
CLI->>User: Prompt for configuration answers
User-->>CLI: Provide answers (Shadcn, Tailwind, TS, ORM)
CLI->>Compat: Check compatibility combinations
Compat-->>CLI: Warnings for incompatibilities
CLI->>User: Display compatibility warnings
CLI->>Installer: runWithRetry(install dependencies)
Installer->>PkgMgr: Execute with retry logic
PkgMgr-->>Installer: Success or failure after retries
CLI->>Setup: Conditional: Install Shadcn UI?
Setup->>FileSystem: Write components.json
Setup->>FileSystem: Generate lib/utils
Setup-->>CLI: Shadcn setup complete
CLI->>Setup: Conditional: Setup ORM?
Setup->>FileSystem: Scaffold Prisma/Drizzle
Setup->>FileSystem: Generate .env file
Setup-->>CLI: ORM setup complete
CLI-->>User: Project creation complete with status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
index.js (1)
376-584:⚠️ Potential issue | 🟠 MajorUse package-manager-specific install commands.
The code currently uses
${packageManager} install --save-devsyntax universally, but this only works for npm. With Yarn or pnpm selected, the installs will fail because:
yarn install <pkg>andpnpm install <pkg>do not add packages to package.json- Yarn requires
yarn add -D <pkg>- pnpm requires
pnpm add -D <pkg>Add a helper function to generate the correct command per package manager and use it across all Biome, Prisma, Drizzle, and Shadcn install sites.
Example helper + usage
+const installCmd = (pm, pkgs, dev = false) => { + switch (pm) { + case 'yarn': + return `yarn add ${dev ? '-D ' : ''}${pkgs}`; + case 'pnpm': + return `pnpm add ${dev ? '-D ' : ''}${pkgs}`; + default: + return `npm install ${dev ? '--save-dev ' : ''}${pkgs}`; + } +}; @@ - await runWithRetry( - `${packageManager} install --save-dev `@biomejs/biome``, - projectPath - ); + await runWithRetry( + installCmd(packageManager, '@biomejs/biome', true), + projectPath + );
🤖 Fix all issues with AI agents
In `@index.js`:
- Around line 54-71: The code incorrectly seeds availablePackageManagers with
'npm', so the "no package manager found" branch never triggers; instead probe
for npm the same way as yarn/pnpm by calling run('npm --version', process.cwd(),
true) and only push 'npm' into availablePackageManagers if that call succeeds
(use the same try/catch pattern around run), leaving availablePackageManagers
initially empty and preserving the existing length check and exit behavior;
update the block that builds availablePackageManagers (refer to
availablePackageManagers and run) to remove the hardcoded 'npm' and detect it
dynamically.
In `@lib/utils.js`:
- Around line 16-44: In runWithRetry, validate or coerce maxRetries to at least
1 and short-circuit with a clear error if caller passed 0 (to avoid lastError
being undefined), ensure all retry and status messages (the initial "Running"
log and the `Attempt ... failed, retrying...` log) are suppressed when silent is
true by wrapping those console.log calls with the same silent check, and when
throwing the final error use lastError safely (e.g., optional chaining or a
fallback message) so the thrown message never assumes lastError is defined;
update references in runWithRetry to implement these guards and silent checks.
In `@test/manual-error-test.js`:
- Around line 79-85: The timeout handler created by setTimeout (which kills
child and resolves with { code: -1, output, errorOutput }) can fire after the
child process already closed, causing a false timeout log and double resolve;
fix by storing the timer id when calling setTimeout and calling
clearTimeout(timer) inside the child process closure handler (e.g., inside the
child.on('close' or 'exit' callback that currently resolves with the real code)
before calling resolve, and ensure the timeout handler checks/avoids resolving
if the promise has already been settled (or rely on clearing the timer to
prevent it).
🧹 Nitpick comments (2)
test/manual-error-test.js (1)
4-12: Useos.tmpdir()instead of hard-coded/tmp.
This keeps the test harness portable across platforms.♻️ Suggested refactor
-import path from 'path'; +import path from 'path'; +import os from 'os'; @@ -const cliPath = path.join(__dirname, '..', 'index.js'); +const cliPath = path.join(__dirname, '..', 'index.js'); +const tmpDir = os.tmpdir(); @@ - const projectPath = path.join('/tmp', projectName); + const projectPath = path.join(tmpDir, projectName); @@ -process.chdir('/tmp'); +process.chdir(tmpDir);Also applies to: 53-58, 127-129
index.js (1)
131-139: Default package manager should align with detected options.
If pnpm isn’t installed, a fixed default can be surprising. Consider defaulting to the first detected option.♻️ Suggested tweak
- default: 'pnpm', + default: availablePackageManagers[0],
Comment on lines
+54
to
+71
| const availablePackageManagers = ['npm']; | ||
|
|
||
| try { | ||
| run("yarn --version", process.cwd(), true); | ||
| availablePackageManagers.push("yarn"); | ||
| } catch (error) { } | ||
| run('yarn --version', process.cwd(), true); | ||
| availablePackageManagers.push('yarn'); | ||
| } catch (error) {} | ||
|
|
||
| try { | ||
| run("pnpm --version", process.cwd(), true); | ||
| availablePackageManagers.push("pnpm"); | ||
| } catch (error) { } | ||
| run('pnpm --version', process.cwd(), true); | ||
| availablePackageManagers.push('pnpm'); | ||
| } catch (error) {} | ||
|
|
||
| // Validate at least npm is available | ||
| if (availablePackageManagers.length === 0) { | ||
| console.log(chalk.bold.red('Error: No package manager found')); | ||
| console.log(chalk.yellow('Please install npm, yarn, or pnpm')); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
Don’t assume npm is installed.
Line 54 seeds availablePackageManagers with npm, so the “no package manager found” check can never trigger. If npm is missing, the CLI will still proceed and later fail on npx.
✅ Suggested fix
- const availablePackageManagers = ['npm'];
+ const availablePackageManagers = [];
+
+ try {
+ run('npm --version', process.cwd(), true);
+ availablePackageManagers.push('npm');
+ } catch (error) {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const availablePackageManagers = ['npm']; | |
| try { | |
| run("yarn --version", process.cwd(), true); | |
| availablePackageManagers.push("yarn"); | |
| } catch (error) { } | |
| run('yarn --version', process.cwd(), true); | |
| availablePackageManagers.push('yarn'); | |
| } catch (error) {} | |
| try { | |
| run("pnpm --version", process.cwd(), true); | |
| availablePackageManagers.push("pnpm"); | |
| } catch (error) { } | |
| run('pnpm --version', process.cwd(), true); | |
| availablePackageManagers.push('pnpm'); | |
| } catch (error) {} | |
| // Validate at least npm is available | |
| if (availablePackageManagers.length === 0) { | |
| console.log(chalk.bold.red('Error: No package manager found')); | |
| console.log(chalk.yellow('Please install npm, yarn, or pnpm')); | |
| process.exit(1); | |
| } | |
| const availablePackageManagers = []; | |
| try { | |
| run('npm --version', process.cwd(), true); | |
| availablePackageManagers.push('npm'); | |
| } catch (error) {} | |
| try { | |
| run('yarn --version', process.cwd(), true); | |
| availablePackageManagers.push('yarn'); | |
| } catch (error) {} | |
| try { | |
| run('pnpm --version', process.cwd(), true); | |
| availablePackageManagers.push('pnpm'); | |
| } catch (error) {} | |
| // Validate at least npm is available | |
| if (availablePackageManagers.length === 0) { | |
| console.log(chalk.bold.red('Error: No package manager found')); | |
| console.log(chalk.yellow('Please install npm, yarn, or pnpm')); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
In `@index.js` around lines 54 - 71, The code incorrectly seeds
availablePackageManagers with 'npm', so the "no package manager found" branch
never triggers; instead probe for npm the same way as yarn/pnpm by calling
run('npm --version', process.cwd(), true) and only push 'npm' into
availablePackageManagers if that call succeeds (use the same try/catch pattern
around run), leaving availablePackageManagers initially empty and preserving the
existing length check and exit behavior; update the block that builds
availablePackageManagers (refer to availablePackageManagers and run) to remove
the hardcoded 'npm' and detect it dynamically.
Comment on lines
+16
to
+44
| export const runWithRetry = async ( | ||
| cmd, | ||
| cwd = process.cwd(), | ||
| silent = false, | ||
| maxRetries = 2 | ||
| ) => { | ||
| let lastError; | ||
|
|
||
| for (let attempt = 1; attempt <= maxRetries; attempt++) { | ||
| try { | ||
| if (!silent) { | ||
| console.log( | ||
| `\n${attempt > 1 ? `Retry ${attempt - 1}: ` : ''}Running: ${cmd}` | ||
| ); | ||
| } | ||
| execSync(cmd, { stdio: silent ? 'pipe' : 'inherit', cwd }); | ||
| return; // Success | ||
| } catch (error) { | ||
| lastError = error; | ||
| if (attempt < maxRetries) { | ||
| console.log(`\nAttempt ${attempt} failed, retrying...`); | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)); // Wait 1 second before retry | ||
| } | ||
| } | ||
| execSync(cmd, { stdio: silent ? "pipe" : "inherit", cwd }); | ||
| } | ||
|
|
||
| throw new Error( | ||
| `Command failed after ${maxRetries} attempts: ${cmd}\nError: ${lastError.message}` | ||
| ); |
There was a problem hiding this comment.
Guard maxRetries and honor silent during retries.
If maxRetries is 0, lastError is undefined and the final throw will crash; retry logs also print even when silent is true.
💡 Suggested fix
export const runWithRetry = async (
cmd,
cwd = process.cwd(),
silent = false,
maxRetries = 2
) => {
+ if (maxRetries < 1) {
+ throw new Error('maxRetries must be at least 1');
+ }
let lastError;
for (let attempt = 1; attempt <= maxRetries; attempt++) {
try {
@@
} catch (error) {
lastError = error;
if (attempt < maxRetries) {
- console.log(`\nAttempt ${attempt} failed, retrying...`);
+ if (!silent) {
+ console.log(`\nAttempt ${attempt} failed, retrying...`);
+ }
await new Promise((resolve) => setTimeout(resolve, 1000)); // Wait 1 second before retry
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const runWithRetry = async ( | |
| cmd, | |
| cwd = process.cwd(), | |
| silent = false, | |
| maxRetries = 2 | |
| ) => { | |
| let lastError; | |
| for (let attempt = 1; attempt <= maxRetries; attempt++) { | |
| try { | |
| if (!silent) { | |
| console.log( | |
| `\n${attempt > 1 ? `Retry ${attempt - 1}: ` : ''}Running: ${cmd}` | |
| ); | |
| } | |
| execSync(cmd, { stdio: silent ? 'pipe' : 'inherit', cwd }); | |
| return; // Success | |
| } catch (error) { | |
| lastError = error; | |
| if (attempt < maxRetries) { | |
| console.log(`\nAttempt ${attempt} failed, retrying...`); | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); // Wait 1 second before retry | |
| } | |
| } | |
| execSync(cmd, { stdio: silent ? "pipe" : "inherit", cwd }); | |
| } | |
| throw new Error( | |
| `Command failed after ${maxRetries} attempts: ${cmd}\nError: ${lastError.message}` | |
| ); | |
| export const runWithRetry = async ( | |
| cmd, | |
| cwd = process.cwd(), | |
| silent = false, | |
| maxRetries = 2 | |
| ) => { | |
| if (maxRetries < 1) { | |
| throw new Error('maxRetries must be at least 1'); | |
| } | |
| let lastError; | |
| for (let attempt = 1; attempt <= maxRetries; attempt++) { | |
| try { | |
| if (!silent) { | |
| console.log( | |
| `\n${attempt > 1 ? `Retry ${attempt - 1}: ` : ''}Running: ${cmd}` | |
| ); | |
| } | |
| execSync(cmd, { stdio: silent ? 'pipe' : 'inherit', cwd }); | |
| return; // Success | |
| } catch (error) { | |
| lastError = error; | |
| if (attempt < maxRetries) { | |
| if (!silent) { | |
| console.log(`\nAttempt ${attempt} failed, retrying...`); | |
| } | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); // Wait 1 second before retry | |
| } | |
| } | |
| } | |
| throw new Error( | |
| `Command failed after ${maxRetries} attempts: ${cmd}\nError: ${lastError.message}` | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In `@lib/utils.js` around lines 16 - 44, In runWithRetry, validate or coerce
maxRetries to at least 1 and short-circuit with a clear error if caller passed 0
(to avoid lastError being undefined), ensure all retry and status messages (the
initial "Running" log and the `Attempt ... failed, retrying...` log) are
suppressed when silent is true by wrapping those console.log calls with the same
silent check, and when throwing the final error use lastError safely (e.g.,
optional chaining or a fallback message) so the thrown message never assumes
lastError is defined; update references in runWithRetry to implement these
guards and silent checks.
Comment on lines
+79
to
+85
| // Handle timeout | ||
| setTimeout(() => { | ||
| child.kill(); | ||
| console.log(chalk.yellow(`⏰ ${projectName}: Timed out`)); | ||
| resolve({ code: -1, output, errorOutput }); | ||
| }, 30000); // 30 second timeout | ||
| }); |
There was a problem hiding this comment.
Clear the timeout when the process closes.
Otherwise the timer can fire after a successful run, logging a false timeout and resolving twice.
🛠️ Suggested fix
- // Handle timeout
- setTimeout(() => {
+ // Handle timeout
+ const timeout = setTimeout(() => {
child.kill();
console.log(chalk.yellow(`⏰ ${projectName}: Timed out`));
resolve({ code: -1, output, errorOutput });
}, 30000); // 30 second timeout
+
+ child.on('close', (code) => {
+ clearTimeout(timeout);
+ ...
+ });🤖 Prompt for AI Agents
In `@test/manual-error-test.js` around lines 79 - 85, The timeout handler created
by setTimeout (which kills child and resolves with { code: -1, output,
errorOutput }) can fire after the child process already closed, causing a false
timeout log and double resolve; fix by storing the timer id when calling
setTimeout and calling clearTimeout(timer) inside the child process closure
handler (e.g., inside the child.on('close' or 'exit' callback that currently
resolves with the real code) before calling resolve, and ensure the timeout
handler checks/avoids resolving if the promise has already been settled (or rely
on clearing the timer to prevent it).
a8750b7 to
da3e7d6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Added robust error handling for package installation and compatibility in the CLI. Now, failed installs are retried, incomplete projects are cleaned up, and users get clear troubleshooting tips. Compatibility warnings are shown before setup.
Related Issue
Fixes #24
Type of Change
Testing
Checklist
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.