feat: suggest proxy-aware retry command when network fails - #1387
Conversation
commit: |
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds centralized proxy support and network error utilities to the Nuxt CLI, including error classification, formatted diagnostics, proxy hints, and retry command generation. CLI bootstrap now configures proxy propagation for child processes. Module, binary download, template initialization, nightly version, and npm dependency requests use structured network error reporting. Template and module API endpoints are centralized, the template request timeout increases to 5000 ms, and comprehensive unit tests cover the new utilities. Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
packages/nuxt-cli/test/unit/utils/network.spec.ts (1)
63-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilent early-return skips mask missing coverage.
Both
reports the current process as proxy-aware when launched with the flag(Lines 63-66) andstays quiet when the proxy is already in use(Lines 162-164) silentlyreturnwith zero assertions when!supportsEnvProxy(), rather than using an explicit skip (e.g.it.skipIf/test.skipIf). On a Node runtime wheresupportsEnvProxy()under- or over-detects support (see the related comment onnetwork.ts), these tests would report green while exercising nothing, hiding exactly that kind of regression.♻️ Suggested fix using Vitest's conditional skip
- it('reports the current process as proxy-aware when launched with the flag', () => { - if (!supportsEnvProxy()) { - return - } + it.skipIf(!supportsEnvProxy())('reports the current process as proxy-aware when launched with the flag', () => { expect(isEnvProxyActive({ NODE_USE_ENV_PROXY: '1' }, [])).toBe(true)Also applies to: 161-168
🤖 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/utils/network.spec.ts` around lines 63 - 72, Replace the silent supportsEnvProxy() early returns in the tests “reports the current process as proxy-aware when launched with the flag” and “stays quiet when the proxy is already in use” with Vitest’s explicit conditional skip mechanism, such as it.skipIf or test.skipIf. Preserve the existing assertions and test behavior when proxy support is available.
🤖 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/commands/init.ts`:
- Line 595: Move the fetchModules() promise creation inside the wantsUserModules
opt-in branch so the modules request is deferred until after confirmation.
Preserve the existing handling for opted-in users while ensuring declining the
prompt never invokes fetchModules() or its failure warning.
In `@packages/nuxt-cli/src/commands/module/add.ts`:
- Line 351: Replace description-only network warnings with proxy-aware
logNetworkError handling for recoverable failures. Update
packages/nuxt-cli/src/commands/module/add.ts lines 351-351,
packages/nuxt-cli/src/commands/module/remove.ts lines 71-71,
packages/nuxt-cli/src/dev/binaries.ts lines 159-159, and
packages/nuxt-cli/src/commands/init.ts lines 596-597 and 717-717, preserving
each existing failure context while emitting NODE_USE_ENV_PROXY retry guidance.
---
Nitpick comments:
In `@packages/nuxt-cli/test/unit/utils/network.spec.ts`:
- Around line 63-72: Replace the silent supportsEnvProxy() early returns in the
tests “reports the current process as proxy-aware when launched with the flag”
and “stays quiet when the proxy is already in use” with Vitest’s explicit
conditional skip mechanism, such as it.skipIf or test.skipIf. Preserve the
existing assertions and test behavior when proxy support is available.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e369c0e8-3ac0-4cd8-a4d3-58b5315c8019
📒 Files selected for processing (10)
packages/nuxt-cli/src/commands/init.tspackages/nuxt-cli/src/commands/module/_utils.tspackages/nuxt-cli/src/commands/module/add.tspackages/nuxt-cli/src/commands/module/remove.tspackages/nuxt-cli/src/commands/module/search.tspackages/nuxt-cli/src/dev/binaries.tspackages/nuxt-cli/src/main.tspackages/nuxt-cli/src/utils/network.tspackages/nuxt-cli/src/utils/starter-templates.tspackages/nuxt-cli/test/unit/utils/network.spec.ts
| // ...or offer to browse and install modules (if not offline nor non-interactive) | ||
| else if (!ctx.args.offline && !ctx.args.preferOffline && !isNonInteractive) { | ||
| const modulesPromise = fetchModules() | ||
| const modulesPromise = fetchModules().catch((err) => { |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Defer the modules request until the user opts in.
fetchModules() starts before the confirmation prompt, so declining module browsing still performs the request and can now emit a warning on failure. Create the promise only after wantsUserModules is true.
🤖 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/commands/init.ts` at line 595, Move the fetchModules()
promise creation inside the wantsUserModules opt-in branch so the modules
request is deferred until after confirmation. Preserve the existing handling for
opted-in users while ensuring declining the prompt never invokes fetchModules()
or its failure warning.
d3280db to
53e6a29
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
2 participants
🔗 Linked issue
resolves #288
resolves #853
closes #419
closes #1089
📚 Description
a lot of issues we face are due to proxy issues with network requests
this PR improves the errors we show in these cases, and give people updated commands to run that will resolve the issue using node's builtin
NODE_USE_ENV_PROXY