fix: retry VMs init for up to 2 min #1612

pujitm · 2025-08-22T16:07:23Z

Resolve #1392 -- where the VMs service would be unavailable following a server restart.

Move VMs Service init to onApplicationBootstrap to avoid blocking application lifecycle prematurely.
Add retry mechanism for VMs Service init

coderabbitai · 2025-08-22T16:07:31Z

Walkthrough

VmsService lifecycle hook changed from OnModuleInit to OnApplicationBootstrap and now includes a scheduled healInitialization retry routine decorated with @timeout to reattempt hypervisor initialization and watcher setup. Corresponding test updated to call onApplicationBootstrap(). No other public signatures changed.

Changes

Cohort / File(s)	Summary of changes
Service lifecycle & healing `api/src/unraid-api/graph/resolvers/vms/vms.service.ts`	Replaced OnModuleInit with OnApplicationBootstrap; renamed `onModuleInit()` → `onApplicationBootstrap()`. Added `@Timeout`-driven `healInitialization(maxRetries = 12, delay = 10_000)` to retry hypervisor initialization and watcher setup with per-attempt logging and delay. Added imports for `OnApplicationBootstrap` and `@Timeout`.
Test alignment `api/src/unraid-api/graph/resolvers/vms/vms.service.spec.ts`	Updated test setup to call `await service.onApplicationBootstrap()` instead of `await service.onModuleInit()`.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as NestJS App
  participant VMS as VmsService
  participant Hyp as Hypervisor/Watcher

  Note over App,VMS: Application bootstrap
  App->>VMS: onApplicationBootstrap()
  VMS->>Hyp: attemptHypervisorInitializationAndWatch()
  alt Init succeeds
    Hyp-->>VMS: Ready
  else Init fails
    VMS-->>VMS: schedule healInitialization (Timeout 10s)
  end

  rect rgb(245,245,255)
  Note right of VMS: healInitialization (retry up to maxRetries)
  loop Retry until success or maxRetries
    VMS->>Hyp: attemptHypervisorInitializationAndWatch()
    alt Success
      Hyp-->>VMS: Ready
      VMS-->>VMS: stop retries
    else Failure
      VMS-->>VMS: wait delay (e.g., 10s)
    end
  end
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

fix: vms now can detect starting of libvirt and start local hypervisor #1356 — Modifies VmsService initialization and lifecycle logic; touches the same init/watch flow.

Suggested reviewers

mdatelle

Poem

I hop through hooks at break of day,
From module dawn to bootstrap gray—
If first try fails, I nibble, wait, retry,
Twelve little hops beneath the sky. 🐇✨

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6664c8b and 439f2f9.

📒 Files selected for processing (1)

api/src/unraid-api/graph/resolvers/vms/vms.service.ts (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

api/src/unraid-api/graph/resolvers/vms/vms.service.ts

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/vms-query-after-restart

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

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

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

Visit our Status Page to check the current availability of CodeRabbit.
Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

claude · 2025-08-22T16:07:37Z

Claude finished @pujitm's task —— View job

Reviewing PR Changes

Read changed files and analyze diff
Review for critical issues in changed lines only
Post review findings

✅ No critical issues found in changes

The diff shows clean improvements:

Line 1-2: Safe import additions for new lifecycle hook and scheduler
Line 15: Proper interface change from OnModuleInit to OnApplicationBootstrap
Line 42-45: Bootstrap method correctly delegates to existing initialization logic
Lines 47-60: Retry logic is sound - 12 retries × 10s = 120s max as intended, with proper logging and early exit on success
Line 199: Test updated correctly to match new lifecycle hook

No race conditions, infinite loops, security issues, or breaking changes detected in the modified lines.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

api/src/unraid-api/graph/resolvers/vms/vms.service.ts (1)

47-56: Retry loop meets the “up to 2 min” goal; consider early-exit and guard against concurrent inits

Behavior matches PR intent: 12 retries × 10s = 120s max.
Minor ergonomics: break immediately after a successful attempt to avoid an unnecessary extra sleep.
Concurrency: multiple callers (bootstrap, timeout, PID watcher) can race initializeHypervisor. If hypervisor.connectOpen isn’t re-entrant, two overlapping attempts could contend. A lightweight guard avoids this.

Apply this small loop tweak:

-    let retries = 1;
-    while (!this.isVmsAvailable && retries <= maxRetries) {
-        this.logger.log(`Attempting to initialize VMs service...attempt ${retries}/${maxRetries}`);
-        await this.attemptHypervisorInitializationAndWatch();
-        await new Promise((resolve) => setTimeout(resolve, delay));
-        retries++;
-    }
+    for (let attempt = 1; !this.isVmsAvailable && attempt <= maxRetries; attempt++) {
+        this.logger.log(`Attempting to initialize VMs service...attempt ${attempt}/${maxRetries}`);
+        await this.attemptHypervisorInitializationAndWatch();
+        if (this.isVmsAvailable) break; // early-exit on success
+        await new Promise((resolve) => setTimeout(resolve, delay));
+    }

And optionally add a simple in-progress guard (outside this hunk) to serialize init:

// class fields
private initializing = false;
private initializingPromise: Promise<void> | null = null;

// wrap initializeHypervisor calls behind a simple latch
private async initializeOnce(): Promise<void> {
  if (this.isVmsAvailable) return;
  if (this.initializingPromise) return this.initializingPromise;

  this.initializing = true;
  this.initializingPromise = (async () => {
    try {
      await this.initializeHypervisor();
      this.isVmsAvailable = true;
    } finally {
      this.initializing = false;
      this.initializingPromise = null;
    }
  })();

  return this.initializingPromise;
}
// then call await this.initializeOnce() from attemptHypervisorInitializationAndWatch and watcher handlers.

api/src/unraid-api/graph/resolvers/vms/vms.service.spec.ts (1)

304-306: Avoid asserting exact error messages in promises; use bare .rejects.toThrow()

Per our testing guidelines, prefer behavior over message wording to reduce brittleness.

Apply this diff:
-        await expect(service.startVm('999')).rejects.toThrow('Failed to set VM state: Invalid UUID');
-        await expect(service.stopVm('999')).rejects.toThrow('Failed to set VM state: Invalid UUID');
+        await expect(service.startVm('999')).rejects.toThrow();
+        await expect(service.stopVm('999')).rejects.toThrow();

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between fd895ca and 6664c8b.

📒 Files selected for processing (2)

api/src/unraid-api/graph/resolvers/vms/vms.service.spec.ts (1 hunks)
api/src/unraid-api/graph/resolvers/vms/vms.service.ts (3 hunks)

🧰 Additional context used

📓 Path-based instructions (6)

**/*.{ts,tsx}