Skip to content

Make AgentManagerImpl.handleCommands resilient to listener exceptions - #13788

Open
Damans227 wants to merge 1 commit into
apache:mainfrom
Damans227:power-state-sync-npe-resilience
Open

Make AgentManagerImpl.handleCommands resilient to listener exceptions#13788
Damans227 wants to merge 1 commit into
apache:mainfrom
Damans227:power-state-sync-npe-resilience

Conversation

@Damans227

Copy link
Copy Markdown
Collaborator

Description

If one listener in handleCommands throws, the whole loop stops and every listener after it never runs. This can starve pingBy() and cause fake ping timeouts.

This wraps each listener call in a try/catch, so one bad listener just logs a warning and the rest still run. Also fixed a trace log nearby that was logging the wrong class (the Pair wrapper instead of the listener).

Also added the missing hostId field to IdsPowerStateSelectSearch. Without it, VMs loaded through that path always showed a null host, even when they had one. Not a crash, just a wrong log message.

Types of changes

  • Bug fix

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • Major

How Has This Been Tested?

Added tests: a throwing listener doesn't block the next one, and existing behavior with only successful listeners is unchanged. Also added tests for the null-hostId case, which had no coverage before.

…steners and ensuring downstream listeners are invoked. Add tests to verify behavior with null hostId and exception handling.
@codecov

codecovBot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 19.65%. Comparing base (4f11707) to head (90ce7bd).

Files with missing linesPatch %Lines
.../main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java0.00%1 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #13788 +/- ##
=========================================
Coverage 19.65% 19.65% - Complexity 19792 19798 +6 
=========================================
Files 6368 6368 Lines 574881 574887 +6 Branches 70351 70351 =========================================
+ Hits 112970 112999 +29 + Misses 449639 449614 -25 - Partials 12272 12274 +2 
FlagCoverage Δ
uitests3.41% <ø> (ø)
unittests20.93% <87.50%> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DaanHooglandDaanHoogland added this to the 4.24.0 milestone Aug 5, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Damans227@DaanHoogland