Merge pull request #234 from OctopusDeploy/huy/sc-124345-add-retry-for-task-await

Add getTasksWithRetry with retry logic for transient network errors

Author: HuyPhanNguyen

src/features/serverTasks/serverTaskWaiter.ts

@@ -1,34 +1,64 @@
+/* eslint-disable no-eq-null */
+/* eslint-disable @typescript-eslint/consistent-type-assertions */
+/* eslint-disable @typescript-eslint/no-non-null-assertion */
 import { Client } from "../..";
 import { ServerTask } from "../../features/serverTasks";
 import { SpaceServerTaskRepository } from "../serverTasks";
 import { ServerTaskRepository } from "../serverTasks";
 
+export interface ServerTaskWaiterOptions {
+    maxRetries?: number; // Default: 3
+    retryBackoffMs?: number; // Initial backoff in ms, default: 5000
+}
+
 export class ServerTaskWaiter {
-    constructor(private readonly client: Client, private readonly spaceName: string) {}
+    private readonly maxRetries: number;
+    private readonly retryBackoffMs: number;
+
+    constructor(private readonly client: Client, private readonly spaceName: string, options?: ServerTaskWaiterOptions) {
+        this.maxRetries = options?.maxRetries ?? 3;
+        this.retryBackoffMs = options?.retryBackoffMs ?? 5000;
+    }
 
     async waitForServerTasksToComplete(
         serverTaskIds: string[],
         statusCheckSleepCycle: number,
         timeout: number,
         pollingCallback?: (serverTask: ServerTask) => void,
-        cancelOnTimeout: boolean = false,
+        cancelOnTimeout: boolean = false
     ): Promise<ServerTask[]> {
         const spaceServerTaskRepository = new SpaceServerTaskRepository(this.client, this.spaceName);
-        const serverTaskRepository = new ServerTaskRepository(this.client)
-
-        return this.waitForTasks(spaceServerTaskRepository, serverTaskRepository, serverTaskIds, statusCheckSleepCycle, timeout, cancelOnTimeout, pollingCallback);
+        const serverTaskRepository = new ServerTaskRepository(this.client);
+
+        return this.waitForTasks(
+            spaceServerTaskRepository,
+            serverTaskRepository,
+            serverTaskIds,
+            statusCheckSleepCycle,
+            timeout,
+            cancelOnTimeout,
+            pollingCallback
+        );
     }
 
     async waitForServerTaskToComplete(
         serverTaskId: string,
         statusCheckSleepCycle: number,
         timeout: number,
         pollingCallback?: (serverTask: ServerTask) => void,
-        cancelOnTimeout: boolean = false,
+        cancelOnTimeout: boolean = false
     ): Promise<ServerTask> {
         const spaceServerTaskRepository = new SpaceServerTaskRepository(this.client, this.spaceName);
-        const serverTaskRepository = new ServerTaskRepository(this.client)
-        const tasks = await this.waitForTasks(spaceServerTaskRepository, serverTaskRepository, [serverTaskId], statusCheckSleepCycle, timeout, cancelOnTimeout, pollingCallback);
+        const serverTaskRepository = new ServerTaskRepository(this.client);
+        const tasks = await this.waitForTasks(
+            spaceServerTaskRepository,
+            serverTaskRepository,
+            [serverTaskId],
+            statusCheckSleepCycle,
+            timeout,
+            cancelOnTimeout,
+            pollingCallback
+        );
         return tasks[0];
     }
 
@@ -61,7 +91,7 @@ export class ServerTaskWaiter {
 
         try {
             while (!stop) {
-                const tasks = await spaceServerTaskRepository.getByIds(serverTaskIds);
+                const tasks = await this.getTasksWithRetry(spaceServerTaskRepository, serverTaskIds);
 
                 const unknownTaskIds = serverTaskIds.filter((id) => tasks.filter((t) => t.Id === id).length == 0);
                 if (unknownTaskIds.length) {
@@ -93,6 +123,7 @@ export class ServerTaskWaiter {
 
                 await sleep(statusCheckSleepCycle);
             }
+
             if (timedOut && cancelOnTimeout && serverTaskIds.length > 0) {
                 await this.cancelTasks(serverTaskRepository, serverTaskIds);
             }
@@ -115,4 +146,72 @@ export class ServerTaskWaiter {
             }
         }
     }
+
+    private async getTasksWithRetry(repository: SpaceServerTaskRepository, taskIds: string[]): Promise<ServerTask[]> {
+        // eslint-disable-next-line @typescript-eslint/init-declarations
+        let lastError: any;
+
+        for (let attempt = 0; attempt <= this.maxRetries; attempt++) {
+            try {
+                return await repository.getByIds(taskIds);
+            } catch (error) {
+                lastError = error;
+                const errorMessage = error instanceof Error ? error.message : String(error);
+
+                const statusCode =
+                    (error as any).StatusCode ||
+                    (typeof (error as any).code === "number" ? (error as any).code : null) ||
+                    (error as any).response?.status ||
+                    (error as any).status;
+
+                const isRetryable = this.isRetryableError(error, statusCode);
+
+                if (!isRetryable) throw error;
+
+                if (attempt === this.maxRetries)
+                    throw new Error(`Failed to connect to Octopus server after ${this.maxRetries} attempts. ` + `Last error: ${errorMessage}`);
+
+                const backoffDelay = this.retryBackoffMs * Math.pow(2, attempt);
+                this.client.warn(
+                    `HTTP request failed (attempt ${attempt + 1}/${this.maxRetries}): ${errorMessage}${
+                        statusCode ? ` [${statusCode}]` : ""
+                    }. Retrying in ${backoffDelay}ms...`
+                );
+                await new Promise((resolve) => setTimeout(resolve, backoffDelay));
+            }
+        }
+
+        // This should never be reached due to throws above, but TypeScript needs it
+        throw lastError;
+    }
+
+    private isRetryableError(error: any, statusCode: number | null): boolean {
+        if (!error) return false;
+
+        if (statusCode && [408, 429, 500, 502, 503, 504].includes(statusCode)) {
+            return true;
+        }
+
+        try {
+            const errorStr = String(error.message || error).toLowerCase();
+            const errorCode = error.code ? String(error.code).toLowerCase() : "";
+            const keywords = [
+                "timeout",
+                "etimedout",
+                "econnreset",
+                "econnrefused",
+                "econnaborted",
+                "enotfound",
+                "eai_again",
+                "epipe",
+                "ehostunreach",
+                "enetunreach",
+                "socket",
+                "network",
+            ];
+            return keywords.some((k) => errorStr.includes(k) || errorCode.includes(k));
+        } catch {
+            return false;
+        }
+    }
 }

src/features/serverTasks/spaceServerTaskRepository.ts

@@ -38,7 +38,10 @@ export class SpaceServerTaskRepository {
                 })
             );
         }
-        return Promise.allSettled(promises).then((result) => flatMap(result, (c) => (c.status == "fulfilled" ? c.value.Items : [])));
+        // Changed from Promise.allSettled to Promise.all
+        // Errors will now propagate instead of being swallowed, allowing retry logic at higher levels
+        const results = await Promise.all(promises);
+        return flatMap(results, (c) => c.Items);
     }
 
     async getDetails(serverTaskId: string): Promise<ServerTaskDetails> {