Skip to content

fix: replace fire-and-forget async with safe InitializeAsync helper (CQ-M3)#416

Merged
laurentiu021 merged 1 commit into
mainfrom
fix/code-quality-remaining
May 18, 2026
Merged

fix: replace fire-and-forget async with safe InitializeAsync helper (CQ-M3)#416
laurentiu021 merged 1 commit into
mainfrom
fix/code-quality-remaining

Conversation

@laurentiu021

@laurentiu021 laurentiu021 commented May 18, 2026

Copy link
Copy Markdown
Owner

Summary

Resolves CQ-M3: fire-and-forget async in 12 ViewModel constructors that could produce unobserved task exceptions.

Changes

ViewModelBase — new InitializeAsync helper

Added a protected static method that wraps async initialization with structured error handling:

  • Catches OperationCanceledException silently (expected during shutdown)
  • Catches all other exceptions and logs them via Serilog
  • Uses CallerMemberName for automatic context in log messages

12 ViewModels updated

Replaced bare discard pattern with the safe helper:

  • AboutViewModel, BatteryHealthViewModel, CleanupViewModel
  • DashboardViewModel, DeepCleanupViewModel, PerformanceViewModel
  • ProcessManagerViewModel, ServicesViewModel, SpeedTestViewModel
  • StartupViewModel, SystemHealthViewModel, WindowsUpdateViewModel

Why this matters

The previous pattern discarded the Task, meaning any exception thrown during async initialization would become an unobserved task exception. On .NET 6+ this doesn't crash the app by default, but the errors are silently lost — making debugging difficult. Now all initialization failures are logged with full context.

Testing

  • Build: 0 errors (main + tests)
  • No behavioral changes — same initialization, better error visibility

Summary by CodeRabbit

  • Refactor
    • Updated async initialization pattern across 12 view models using new centralized helper in ViewModelBase.
    • Enhanced error logging during application startup sequences.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 18, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 72bac282-ccdf-47ad-82de-92199a8eaba6

📥 Commits

Reviewing files that changed from the base of the PR and between 0dbd821 and f5a7dd8.

📒 Files selected for processing (14)
  • CHANGELOG.md
  • SysManager/SysManager/ViewModels/AboutViewModel.cs
  • SysManager/SysManager/ViewModels/BatteryHealthViewModel.cs
  • SysManager/SysManager/ViewModels/CleanupViewModel.cs
  • SysManager/SysManager/ViewModels/DashboardViewModel.cs
  • SysManager/SysManager/ViewModels/DeepCleanupViewModel.cs
  • SysManager/SysManager/ViewModels/PerformanceViewModel.cs
  • SysManager/SysManager/ViewModels/ProcessManagerViewModel.cs
  • SysManager/SysManager/ViewModels/ServicesViewModel.cs
  • SysManager/SysManager/ViewModels/SpeedTestViewModel.cs
  • SysManager/SysManager/ViewModels/StartupViewModel.cs
  • SysManager/SysManager/ViewModels/SystemHealthViewModel.cs
  • SysManager/SysManager/ViewModels/ViewModelBase.cs
  • SysManager/SysManager/ViewModels/WindowsUpdateViewModel.cs
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build & unit tests
  • GitHub Check: Analyze (csharp)
🔇 Additional comments (14)
CHANGELOG.md (1)

9-21: LGTM!

SysManager/SysManager/ViewModels/ViewModelBase.cs (1)

6-6: LGTM!

Also applies to: 19-38

SysManager/SysManager/ViewModels/AboutViewModel.cs (1)

50-50: LGTM!

SysManager/SysManager/ViewModels/BatteryHealthViewModel.cs (1)

26-26: LGTM!

SysManager/SysManager/ViewModels/CleanupViewModel.cs (1)

56-56: LGTM!

SysManager/SysManager/ViewModels/DashboardViewModel.cs (1)

58-58: LGTM!

SysManager/SysManager/ViewModels/DeepCleanupViewModel.cs (1)

63-63: LGTM!

SysManager/SysManager/ViewModels/PerformanceViewModel.cs (1)

57-57: LGTM!

SysManager/SysManager/ViewModels/ProcessManagerViewModel.cs (1)

36-36: LGTM!

SysManager/SysManager/ViewModels/ServicesViewModel.cs (1)

39-39: LGTM!

SysManager/SysManager/ViewModels/SpeedTestViewModel.cs (1)

43-43: LGTM!

SysManager/SysManager/ViewModels/StartupViewModel.cs (1)

33-33: LGTM!

SysManager/SysManager/ViewModels/SystemHealthViewModel.cs (1)

53-53: LGTM!

SysManager/SysManager/ViewModels/WindowsUpdateViewModel.cs (1)

38-38: LGTM!


📝 Walkthrough

Walkthrough

A new InitializeAsync helper in ViewModelBase provides structured error handling for async initialization by wrapping tasks, catching exceptions, and logging via Serilog instead of creating unobserved exceptions. Twelve ViewModels are updated to use this helper in their constructors instead of the fire-and-forget pattern.

Changes

Safe async ViewModel initialization

Layer / File(s) Summary
ViewModelBase initialization helper
SysManager/SysManager/ViewModels/ViewModelBase.cs
ViewModelBase adds using Serilog; and a new protected static InitializeAsync(Func<Task> asyncAction, [CallerMemberName] string callerName) method. The helper awaits the task, suppresses OperationCanceledException, and logs other exceptions with caller member name.
ViewModel migrations
SysManager/SysManager/ViewModels/AboutViewModel.cs, BatteryHealthViewModel.cs, CleanupViewModel.cs, DashboardViewModel.cs, DeepCleanupViewModel.cs, PerformanceViewModel.cs, ProcessManagerViewModel.cs, ServicesViewModel.cs, SpeedTestViewModel.cs, StartupViewModel.cs, SystemHealthViewModel.cs, WindowsUpdateViewModel.cs
All twelve ViewModels update their constructors from _ = InitAsync() (or _ = LoadHistoryAsync() / _ = LoadHealthScoreAsync()) to calling InitializeAsync(InitAsync) (or the respective async method name), delegating lifecycle management to the new helper.
Changelog documentation
CHANGELOG.md
"Unreleased" section documents the new InitializeAsync helper for safe async initialization and notes the migration of twelve ViewModels away from the fire-and-forget pattern.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • laurentiu021/SystemManager#372: Modifies async initialization patterns in ProcessManagerViewModel, complementary to the constructor initialization refactoring in this PR.

Poem

A rabbit hops through async code,
With errors logged, no silent load,
Fire-and-forget now has a guard,
Serilog watches—no task uncharred! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.76% 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 name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: replacing fire-and-forget async patterns with a safe InitializeAsync helper across 12 ViewModels, which directly matches the primary objective and changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/code-quality-remaining

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

Comment on lines +34 to +37
catch (Exception ex)
{
Log.Error(ex, "Unhandled exception in async initialization of {Caller}", callerName);
}
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
SysManager/SysManager/ViewModels/ViewModelBase.cs 42.85% 4 Missing ⚠️
...Manager/SysManager/ViewModels/ServicesViewModel.cs 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@laurentiu021
laurentiu021 merged commit c6c335b into main May 18, 2026
5 checks passed
@laurentiu021
laurentiu021 deleted the fix/code-quality-remaining branch May 18, 2026 08:34
laurentiu021 added a commit that referenced this pull request May 22, 2026
…route DNS (#431)

## Summary

**Batch 10** — final batch of the QA bug fix series.

### Issue #414 — Service WaitForStatus can hang
- Wrapped \WaitForStatus\ in try-catch for \TimeoutException\
- Converts to descriptive \InvalidOperationException\ with user-friendly
message
- No more unhandled crash when a service takes >30s to start/stop

### Issue #415 — Snapshot not persisted to disk
- Added \SaveSnapshot\/\LoadSnapshot\ to \PerformanceService\
- Persists \OriginalSnapshot\ as JSON in
\%LOCALAPPDATA%\\SysManager\\performance-snapshot.json\
- \EnsureSnapshotAsync\ now: loads from disk → falls back to live
capture → saves to disk
- Snapshot survives app restarts — Restore All works after reopen

### Issue #416 — Traceroute DNS fire-and-forget race condition
- Replaced fire-and-forget \Task.Run\ DNS lookup with awaited
\Dns.GetHostEntryAsync\
- Added 1.5s timeout via
\CancellationTokenSource.CreateLinkedTokenSource\
- \hop.HostName\ is now populated BEFORE emission via \HopCompleted\
- No more race condition where UI renders '*' and never updates

### Testing
- Build: 0 errors

Closes #414, Closes #415, Closes #416

Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
laurentiu021 added a commit that referenced this pull request May 22, 2026
Adds CHANGELOG entries for all 9 releases from the QA bug fix session:

- **v0.28.16** — Dispose lifecycle (#395, #410)
- **v0.28.17** — CTS disposal + bare catch (#396, #413)
- **v0.28.18** — Input validation + null checks (#397, #398)
- **v0.28.19** — JSON error handling (#400)
- **v0.28.20** — Drive scanning + cache eviction + ConfigureAwait (#401,
#402, #403)
- **v0.28.21** — Audit logging + error messages (#405, #407)
- **v0.28.22** — SHA256 verification (#408, #409)
- **v0.28.23** — Service timeout + snapshot persist + traceroute DNS
(#414, #415, #416)
- **v0.28.24** — Accessibility (#411)

18 bugs fixed in total.

Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
laurentiu021 added a commit that referenced this pull request May 22, 2026
…CQ-M3) (#416)

Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
Sign up for free to 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.

3 participants