fix(pve): Fix connection issues and add more error handlings#1081
fix(pve): Fix connection issues and add more error handlings#1081GT-610 merged 3 commits intolollipopkit:mainfrom
Conversation
Added a detailed display of loading steps during the PVE connection process, including stages such as establishing an SSH tunnel, authentication, and data retrieval Also optimized the sorting of PVE storage content and the logic for handling connection errors
…tication Added error handling for PVE servers when two-factor authentication is enabled, along with relevant error types and localized prompts
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (15)
📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds a new Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/data/provider/pve.dart (1)
11-11:⚠️ Potential issue | 🟡 MinorClear the remaining
flutter analysiswarnings in this file.Line 11's locale import is unused now, and Lines 136-138 assign
newUrlwithout ever reading it. Both warnings are already reported by the current analysis job.Also applies to: 136-138
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/data/provider/pve.dart` at line 11, Remove the unused locale import and eliminate the dead assignment to newUrl: delete the import of 'package:server_box/core/extension/context/locale.dart' near the top of lib/data/provider/pve.dart, and in the block where newUrl is assigned (previously lines ~136-138) either remove the newUrl variable/assignment entirely or refactor to actually use newUrl in the subsequent logic; ensure no other symbols in pve.dart reference that import or variable and re-run flutter analyze to confirm warnings are cleared.lib/view/page/pve.dart (1)
72-79:⚠️ Potential issue | 🟠 MajorRetry the PVE bootstrap here, not just
list().Line 77 only re-runs
list(), butlist()exits untilisConnectedis true. That makes the refresh button a no-op for forwarding/login/bootstrap failures and leaves the page stuck on the error state until it is reopened.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/view/page/pve.dart` around lines 72 - 79, The retry button currently only calls _notifier.list(), which is ineffective when the PVE bootstrap/login/connection hasn't completed; change the handler in the Btn.icon onTap to invoke the PVE bootstrap/reconnect routine (the method that initiates connection/login/bootstrap on the notifier — e.g., _notifier.bootstrap() or whatever method triggers the full bootstrap flow) instead of just _notifier.list(), and still call _initRefreshTimer() afterwards so the refresh timer is reinitialized; reference the Btn.icon onTap, _notifier.list(), _initRefreshTimer(), and the notifier's bootstrap/connect method when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/data/provider/pve.dart`:
- Around line 59-65: The provider's build method (PveState build(Spi spiParam))
wrongly uses ref.read(serverProvider(spiParam.id)) and immediately returns a
PveErr when serverState.client == null, preventing rebuild when genClient()
later sets client; change ref.read(...) to
ref.watch(serverProvider(spiParam.id)) so the provider will rebuild when
ServerState.client becomes available and replace the immediate error return with
a non-error/loading PveState (or defer error only for real failures) so a
transient null SSH client doesn't become a permanent PVE error.
In `@lib/view/page/pve.dart`:
- Around line 142-148: Replace the hardcoded English strings in
_buildLoading(PveLoadingStep step) (the message variable for
PveLoadingStep.forwarding, PveLoadingStep.loggingIn,
PveLoadingStep.fetchingData, and the default) with localized strings from the
app l10n (e.g., use context.l10n or AppLocalizations.of(context).<key>); if the
keys do not yet exist, add appropriate keys to the ARB/localization resource
files (forwarding, loggingIn, fetchingData, connecting) and regenerate
localization, then reference those keys in _buildLoading so the UI uses
translated messages instead of hardcoded text.
---
Outside diff comments:
In `@lib/data/provider/pve.dart`:
- Line 11: Remove the unused locale import and eliminate the dead assignment to
newUrl: delete the import of
'package:server_box/core/extension/context/locale.dart' near the top of
lib/data/provider/pve.dart, and in the block where newUrl is assigned
(previously lines ~136-138) either remove the newUrl variable/assignment
entirely or refactor to actually use newUrl in the subsequent logic; ensure no
other symbols in pve.dart reference that import or variable and re-run flutter
analyze to confirm warnings are cleared.
In `@lib/view/page/pve.dart`:
- Around line 72-79: The retry button currently only calls _notifier.list(),
which is ineffective when the PVE bootstrap/login/connection hasn't completed;
change the handler in the Btn.icon onTap to invoke the PVE bootstrap/reconnect
routine (the method that initiates connection/login/bootstrap on the notifier —
e.g., _notifier.bootstrap() or whatever method triggers the full bootstrap flow)
instead of just _notifier.list(), and still call _initRefreshTimer() afterwards
so the refresh timer is reinitialized; reference the Btn.icon onTap,
_notifier.list(), _initRefreshTimer(), and the notifier's bootstrap/connect
method when making the change.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0531208e-614d-4cd7-8d43-f3cefe22a132
⛔ Files ignored due to path filters (16)
lib/generated/l10n/l10n.dartis excluded by!**/generated/**lib/generated/l10n/l10n_de.dartis excluded by!**/generated/**lib/generated/l10n/l10n_en.dartis excluded by!**/generated/**lib/generated/l10n/l10n_es.dartis excluded by!**/generated/**lib/generated/l10n/l10n_fr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_id.dartis excluded by!**/generated/**lib/generated/l10n/l10n_it.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ja.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ko.dartis excluded by!**/generated/**lib/generated/l10n/l10n_nl.dartis excluded by!**/generated/**lib/generated/l10n/l10n_pt.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ru.dartis excluded by!**/generated/**lib/generated/l10n/l10n_tr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_uk.dartis excluded by!**/generated/**lib/generated/l10n/l10n_zh.dartis excluded by!**/generated/**pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
lib/data/model/app/error.dartlib/data/model/server/pve.dartlib/data/provider/container.g.dartlib/data/provider/pve.dartlib/data/provider/pve.freezed.dartlib/data/provider/pve.g.dartlib/data/provider/server/all.g.dartlib/data/provider/server/single.g.dartlib/data/provider/systemd.g.dartlib/data/res/github_id.dartlib/l10n/app_en.arblib/l10n/app_zh.arblib/l10n/app_zh_tw.arblib/view/page/pve.dart
…ation - Added the `pvePwd` field to the `ServerCustom` model - Added a PVE password input field to the edit page (displayed only during key-based authentication) - Updated multilingual files to support PVE-related loading states and password prompts - Optimized PVE connection logic to support password verification during key-based authentication
Resolve #644.
Summary by CodeRabbit
New Features
Bug Fixes