Skip to content

refactor: report first-run and lock failures as actionable errors - #1445

Merged
danielroe merged 1 commit into
mainfrom
refactor/actionable
Aug 12, 2026
Merged

refactor: report first-run and lock failures as actionable errors#1445
danielroe merged 1 commit into
mainfrom
refactor/actionable

Conversation

@danielroe

Copy link
Copy Markdown
Member

🔗 Linked issue

📚 Description

this adopts the ActionableError from #1444 to strip stacktraces from errors which don't need them, in more places

@pkg-pr-new

pkg-pr-newBot commented Aug 12, 2026

Copy link
Copy Markdown
  • nuxt-cli-playground

    npm i https://pkg.pr.new/create-nuxt@1445
    
    npm i https://pkg.pr.new/nuxi@1445
    
    npm i https://pkg.pr.new/@nuxt/cli@1445
    

commit: 96ee7bb

@codspeed-hq

codspeed-hqBot commented Aug 12, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 2 untouched benchmarks


Comparing refactor/actionable (96ee7bb) with main (84c9cea)

Open in CodSpeed

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The CLI now uses ActionableError for lock conflicts and several validation and loading failures. Lock messages can identify the affected output directory. Interactive-session helpers now reside in stdout.ts, while the duplicate console helper was removed. Unit tests verify formatted lock errors and output-directory details.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 44.44% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly summarizes the main change: reporting first-run and lock failures as actionable errors.
Description check✅ PassedThe description directly explains the adoption of ActionableError to remove unnecessary stack traces.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/actionable

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/nuxt-cli/test/unit/commands/analyze.spec.ts (1)

158-162: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Protect the ActionableError type contract in both lock-conflict tests.

The current assertions pass when a generic Error contains the expected message.

  • packages/nuxt-cli/test/unit/commands/analyze.spec.ts#L158-L162: assert that the rejection is an ActionableError.
  • packages/nuxt-cli/test/unit/commands/build.spec.ts#L128-L132: assert that the rejection is an ActionableError.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nuxt-cli/test/unit/commands/analyze.spec.ts` around lines 158 - 162,
Strengthen the lock-conflict rejection assertions in
packages/nuxt-cli/test/unit/commands/analyze.spec.ts lines 158-162 and
packages/nuxt-cli/test/unit/commands/build.spec.ts lines 128-132 to require an
ActionableError instance, not merely an error message containing “locked”;
preserve the existing formatLockError assertions and lock-conflict behavior in
both tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nuxt-cli/src/utils/lockfile.ts`:
- Around line 335-344: Update formatLockError to distinguish an omitted
outputDir from an explicitly empty string: treat only undefined as absent, and
render an empty outputDir as "." in the writing-message path while preserving
existing non-empty paths.
---
Nitpick comments:
In `@packages/nuxt-cli/test/unit/commands/analyze.spec.ts`:
- Around line 158-162: Strengthen the lock-conflict rejection assertions in
packages/nuxt-cli/test/unit/commands/analyze.spec.ts lines 158-162 and
packages/nuxt-cli/test/unit/commands/build.spec.ts lines 128-132 to require an
ActionableError instance, not merely an error message containing “locked”;
preserve the existing formatLockError assertions and lock-conflict behavior in
both tests.
🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 17dc8537-bcc2-4def-95d9-4d5b405266e1

📥 Commits

Reviewing files that changed from the base of the PR and between 38a63a6 and 3344516.

📒 Files selected for processing (17)
  • packages/nuxt-cli/src/commands/analyze.ts
  • packages/nuxt-cli/src/commands/build.ts
  • packages/nuxt-cli/src/commands/devtools.ts
  • packages/nuxt-cli/src/commands/test.ts
  • packages/nuxt-cli/src/dev/cert.ts
  • packages/nuxt-cli/src/dev/listen.ts
  • packages/nuxt-cli/src/dev/preflight.ts
  • packages/nuxt-cli/src/dev/takeover.ts
  • packages/nuxt-cli/src/dev/utils.ts
  • packages/nuxt-cli/src/utils/config.ts
  • packages/nuxt-cli/src/utils/console.ts
  • packages/nuxt-cli/src/utils/kit.ts
  • packages/nuxt-cli/src/utils/lockfile.ts
  • packages/nuxt-cli/src/utils/stdout.ts
  • packages/nuxt-cli/test/unit/commands/analyze.spec.ts
  • packages/nuxt-cli/test/unit/commands/build.spec.ts
  • packages/nuxt-cli/test/unit/lockfile.spec.ts
💤 Files with no reviewable changes (1)
  • packages/nuxt-cli/src/utils/console.ts

Comment on lines +335 to +344
export function formatLockError(info: LockInfo, options: { outputDir?: string } = {}): string {
const isWindows = process.platform === 'win32'
const killCmd = isWindows ? `taskkill /PID ${info.pid} /F` : `kill ${info.pid}`
const label = info.command === 'dev' ? 'dev server' : 'build'

const lines = [
'',
`Another Nuxt ${label} is already running:`,
options.outputDir
? `Another Nuxt ${label} is already writing to ${options.outputDir}:`
: `Another Nuxt ${label} is already running:`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve output context when outputDir is empty.

If a caller passes '', this truthiness check emits the generic “already running” message. Treat undefined as the absent-value case and render an empty relative path as ..

Proposed fix
- options.outputDir- ? `Another Nuxt ${label} is already writing to ${options.outputDir}:`+ options.outputDir !== undefined+ ? `Another Nuxt ${label} is already writing to ${options.outputDir || '.'}:`
📝 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
exportfunctionformatLockError(info: LockInfo,options: {outputDir?: string}={}): string{
constisWindows=process.platform==='win32'
constkillCmd=isWindows ? `taskkill /PID ${info.pid} /F` : `kill ${info.pid}`
constlabel=info.command==='dev' ? 'dev server' : 'build'
constlines=[
'',
`Another Nuxt ${label} is already running:`,
options.outputDir
? `Another Nuxt ${label} is already writing to ${options.outputDir}:`
: `Another Nuxt ${label} is already running:`,
exportfunctionformatLockError(info: LockInfo,options: {outputDir?: string}={}): string{
constisWindows=process.platform==='win32'
constkillCmd=isWindows ? `taskkill /PID ${info.pid} /F` : `kill ${info.pid}`
constlabel=info.command==='dev' ? 'dev server' : 'build'
constlines=[
'',
options.outputDir!==undefined
? `Another Nuxt ${label} is already writing to ${options.outputDir||'.'}:`
: `Another Nuxt ${label} is already running:`,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nuxt-cli/src/utils/lockfile.ts` around lines 335 - 344, Update
formatLockError to distinguish an omitted outputDir from an explicitly empty
string: treat only undefined as absent, and render an empty outputDir as "." in
the writing-message path while preserving existing non-empty paths.

@danielroe
danielroe added this pull request to the merge queueAug 12, 2026
@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to a conflict with the base branch Aug 12, 2026
Base automatically changed from feat/preflight to mainAugust 12, 2026 12:31
@danielroe
danielroe added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit 8d71487Aug 12, 2026
19 checks passed
@danielroe
danielroe deleted the refactor/actionable branch August 12, 2026 13:19
@github-actionsgithub-actionsBot mentioned this pull request Aug 12, 2026
This was referenced Aug 24, 2026
Sign up for freeto 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.

1 participant

@danielroe