From 558ea84880d29528825924cdca680a3fb46ce77b Mon Sep 17 00:00:00 2001
From: Florent BEAUCHAMP <florent.beauchamp@vates.fr>
Date: Fri, 27 Sep 2024 09:11:04 +0200
Subject: [PATCH] feat(backups):  improve health check error messages (#8016)

- waitObjectState already throw if timeout is reached
- gives more details to help user understand the issue
  - timeout reached while waiting for  ${refOrUuid} to be running
  - timeout reached while waiting for  ${refOrUuid} to report the
    driver version through the xen tools.
---
 .../backups/HealthCheckVmBackup.mjs           | 24 ++++++-------------
 @xen-orchestra/xapi/index.mjs                 | 11 +++++++--
 CHANGELOG.unreleased.md                       |  3 +++
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/@xen-orchestra/backups/HealthCheckVmBackup.mjs b/@xen-orchestra/backups/HealthCheckVmBackup.mjs
index eff922f8a75..41b9e96952b 100644
--- a/@xen-orchestra/backups/HealthCheckVmBackup.mjs
+++ b/@xen-orchestra/backups/HealthCheckVmBackup.mjs
@@ -49,26 +49,25 @@ export class HealthCheckVmBackup {
         }
 
         // wait for the 'Running' event to be really stored in local xapi object cache
+
         restoredVm = await xapi.waitObjectState(restoredVm.$ref, vm => vm.power_state === 'Running', {
           timeout: remainingTimeout,
+          timeoutMessage: refOrUuid =>
+            `local xapi  did not get Running state for VM ${refOrUuid} after ${timeout / 1000} second`,
         })
 
         const running = new Date()
         remainingTimeout -= running - started
 
-        if (remainingTimeout < 0) {
-          throw new Error(`local xapi  did not get Running state for VM ${restoredId} after ${timeout / 1000} second`)
-        }
         // wait for the guest tool version to be defined
         await xapi.waitObjectState(restoredVm.guest_metrics, gm => gm?.PV_drivers_version?.major !== undefined, {
           timeout: remainingTimeout,
+          timeoutMessage: refOrUuid =>
+            `timeout reached while waiting for ${refOrUuid} to report the driver version through the Xen tools. Please check or update the Xen tools.`,
         })
 
         const guestToolsReady = new Date()
         remainingTimeout -= guestToolsReady - running
-        if (remainingTimeout < 0) {
-          throw new Error(`local xapi  did not get he guest tools check ${restoredId} after ${timeout / 1000} second`)
-        }
 
         if (waitForScript) {
           const startedRestoredVm = await xapi.waitObjectState(
@@ -79,19 +78,10 @@ export class HealthCheckVmBackup {
                 vm.xenstore_data['vm-data/xo-backup-health-check'] === 'failure'),
             {
               timeout: remainingTimeout,
+              timeoutMessage: refOrUuid =>
+                `timeout reached while waiting for ${refOrUuid} to report the startup script execution.`,
             }
           )
-          const scriptOk = new Date()
-          remainingTimeout -= scriptOk - guestToolsReady
-          if (remainingTimeout < 0) {
-            throw new Error(
-              `Backup health check script did not update vm-data/xo-backup-health-check of ${restoredId} after ${
-                timeout / 1000
-              } second, got ${
-                startedRestoredVm.xenstore_data['vm-data/xo-backup-health-check']
-              } instead of 'success' or 'failure'`
-            )
-          }
 
           if (startedRestoredVm.xenstore_data['vm-data/xo-backup-health-check'] !== 'success') {
             const message = startedRestoredVm.xenstore_data['vm-data/xo-backup-health-check-error']
diff --git a/@xen-orchestra/xapi/index.mjs b/@xen-orchestra/xapi/index.mjs
index ad728d8b125..b4b325a6232 100644
--- a/@xen-orchestra/xapi/index.mjs
+++ b/@xen-orchestra/xapi/index.mjs
@@ -231,7 +231,14 @@ export class Xapi extends Base {
 
   // wait for an object to be in a specified state
 
-  waitObjectState(refOrUuid, predicate, { timeout } = {}) {
+  waitObjectState(
+    refOrUuid,
+    predicate,
+    {
+      timeout,
+      timeoutMessage = refOrUuid => `waitObjectState: timeout reached before ${refOrUuid} in expected state`,
+    } = {}
+  ) {
     return new Promise((resolve, reject) => {
       const object = this.getObject(refOrUuid, undefined)
       if (object !== undefined && predicate(object)) {
@@ -248,7 +255,7 @@ export class Xapi extends Base {
       })
 
       if (timeout !== undefined) {
-        const error = new Error(`waitObjectState: timeout reached before ${refOrUuid} in expected state`)
+        const error = new Error(timeoutMessage(refOrUuid))
         timeoutHandle = setTimeout(() => {
           stop()
           reject(error)
diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
index 7d61e6fbece..498050711e2 100644
--- a/CHANGELOG.unreleased.md
+++ b/CHANGELOG.unreleased.md
@@ -19,6 +19,7 @@
   - `rest get --output $file` now displays progress information during download
   - `rest post` and `rest put` now accept `--input $file` to upload a file and display progress information
 - [Backup] Detect invalid VDI exports that are incorrectly reported as successful by XAPI
+- [Backup/HealthCheck] Improve error messages on health check timeout (PR [#8016](https://github.com/vatesfr/xen-orchestra/pull/8016))
 
 ### Bug fixes
 
@@ -40,8 +41,10 @@
 
 <!--packages-start-->
 
+- @xen-orchestra/backups minor
 - @xen-orchestra/web minor
 - @xen-orchestra/web-core minor
+- @xen-orchestra/xapi minor
 - xen-api minor
 - xo-cli minor
 - xo-server minor