fix: resolve instead of reject in case of timeout

Author: gagik

src/deferred-promise.ts

@@ -1,14 +1,19 @@
-type DeferredPromiseOptions = {
+type DeferredPromiseOptions<T> = {
     timeout?: number;
+    onTimeout?: (resolve: (value: T) => void, reject: (reason: Error) => void) => void;
 };
 
 /** Creates a promise and exposes its resolve and reject methods, with an optional timeout. */
+// eslint-disable-next-line @typescript-eslint/no-empty-object-type
 export class DeferredPromise<T> extends Promise<T> {
     resolve!: (value: T) => void;
     reject!: (reason: unknown) => void;
     private timeoutId?: NodeJS.Timeout;
 
-    constructor(resolver: (resolve: (value: T) => void, reject: (reason: Error) => void) => void, timeout?: number) {
+    constructor(
+        executor: (resolve: (value: T) => void, reject: (reason: Error) => void) => void,
+        { timeout, onTimeout }: DeferredPromiseOptions<T> = {}
+    ) {
         let resolveFn: (value: T) => void;
         let rejectFn: (reason?: unknown) => void;
 
@@ -24,12 +29,12 @@ export class DeferredPromise<T> extends Promise<T> {
 
         if (timeout !== undefined) {
             this.timeoutId = setTimeout(() => {
-                this.reject(new Error("Promise timed out"));
+                onTimeout?.(this.resolve, this.reject);
             }, timeout);
         }
 
-        if (resolver) {
-            resolver(
+        if (executor) {
+            executor(
                 (value: T) => {
                     if (this.timeoutId) clearTimeout(this.timeoutId);
                     this.resolve(value);
@@ -42,7 +47,7 @@ export class DeferredPromise<T> extends Promise<T> {
         }
     }
 
-    static fromPromise<T>(promise: Promise<T>, options: DeferredPromiseOptions = {}): DeferredPromise<T> {
+    static fromPromise<T>(promise: Promise<T>, options: DeferredPromiseOptions<T> = {}): DeferredPromise<T> {
         return new DeferredPromise<T>((resolve, reject) => {
             promise
                 .then((value) => {
@@ -51,6 +56,6 @@ export class DeferredPromise<T> extends Promise<T> {
                 .catch((reason) => {
                     reject(reason as Error);
                 });
-        }, options.timeout);
+        }, options);
     }
 }

src/logger.ts

@@ -18,6 +18,7 @@ export const LogId = {
     telemetryEmitStart: mongoLogId(1_002_003),
     telemetryEmitSuccess: mongoLogId(1_002_004),
     telemetryDeviceIdFailure: mongoLogId(1_002_005),
+    telemetryDeviceIdTimeout: mongoLogId(1_002_006),
 
     toolExecute: mongoLogId(1_003_001),
     toolExecuteFailure: mongoLogId(1_003_002),

src/telemetry/telemetry.ts

@@ -39,13 +39,14 @@ export class Telemetry {
     }
 
     private async start(): Promise<void> {
-        this.deviceIdPromise = DeferredPromise.fromPromise(this.getDeviceId());
-        try {
-            this.commonProperties.device_id = await this.deviceIdPromise;
-        } catch (error) {
-            logger.debug(LogId.telemetryDeviceIdFailure, "telemetry", String(error));
-            this.commonProperties.device_id = "unknown";
-        }
+        this.deviceIdPromise = DeferredPromise.fromPromise(this.getDeviceId(), {
+            timeout: DEVICE_ID_TIMEOUT,
+            onTimeout: (resolve) => {
+                resolve("unknown");
+                logger.debug(LogId.telemetryDeviceIdTimeout, "telemetry", "Device ID retrieval timed out");
+            },
+        });
+        this.commonProperties.device_id = await this.deviceIdPromise;
 
         this.isBufferingEvents = false;
     }
@@ -65,7 +66,7 @@ export class Telemetry {
                 return this.commonProperties.device_id;
             }
 
-            const originalId = await DeferredPromise.fromPromise(machineId(true), { timeout: DEVICE_ID_TIMEOUT });
+            const originalId = await machineId(true);
 
             // Create a hashed format from the all uppercase version of the machine ID
             // to match it exactly with the denisbrodbeck/machineid library that Atlas CLI uses.

tests/unit/deferred-promise.test.ts

@@ -1,6 +1,13 @@
 import { DeferredPromise } from "../../src/deferred-promise.js";
 
 describe("DeferredPromise", () => {
+    beforeEach(() => {
+        jest.useFakeTimers();
+    });
+    afterEach(() => {
+        jest.useRealTimers();
+    });
+
     it("should resolve with the correct value", async () => {
         const deferred = new DeferredPromise<string>((resolve) => {
             resolve("resolved value");
@@ -18,44 +25,47 @@ describe("DeferredPromise", () => {
     });
 
     it("should timeout if not resolved or rejected within the specified time", async () => {
-        const deferred = new DeferredPromise<string>(() => {
-            // Do not resolve or reject
-        }, 10);
+        const deferred = new DeferredPromise<string>(
+            () => {
+                // Do not resolve or reject
+            },
+            { timeout: 100, onTimeout: (resolve, reject) => reject(new Error("Promise timed out")) }
+        );
+
+        jest.advanceTimersByTime(100);
 
         await expect(deferred).rejects.toThrow("Promise timed out");
     });
 
     it("should clear the timeout when resolved", async () => {
-        jest.useFakeTimers();
-
-        const deferred = new DeferredPromise<string>((resolve) => {
-            setTimeout(() => resolve("resolved value"), 100);
-        }, 200);
+        const deferred = new DeferredPromise<string>(
+            (resolve) => {
+                setTimeout(() => resolve("resolved value"), 100);
+            },
+            { timeout: 200 }
+        );
 
         const promise = deferred.then((value) => {
             expect(value).toBe("resolved value");
         });
 
         jest.advanceTimersByTime(100);
         await promise;
-
-        jest.useRealTimers();
     });
 
     it("should clear the timeout when rejected", async () => {
-        jest.useFakeTimers();
-
-        const deferred = new DeferredPromise<string>((_, reject) => {
-            setTimeout(() => reject(new Error("rejected error")), 100);
-        }, 200);
+        const deferred = new DeferredPromise<string>(
+            (_, reject) => {
+                setTimeout(() => reject(new Error("rejected error")), 100);
+            },
+            { timeout: 200, onTimeout: (resolve, reject) => reject(new Error("Promise timed out")) }
+        );
 
         const promise = deferred.catch((error) => {
             expect(error).toEqual(new Error("rejected error"));
         });
 
         jest.advanceTimersByTime(100);
         await promise;
-
-        jest.useRealTimers();
     });
 });

tests/unit/telemetry.test.ts

@@ -315,9 +315,9 @@ describe("Telemetry", () => {
             expect(telemetry.getCommonProperties().device_id).toBe("unknown");
             expect(telemetry["isBufferingEvents"]).toBe(false);
             expect(loggerSpy).toHaveBeenCalledWith(
-                LogId.telemetryDeviceIdFailure,
+                LogId.telemetryDeviceIdTimeout,
                 "telemetry",
-                "Error: Promise timed out"
+                "Device ID retrieval timed out"
             );
         });
     });