Throw instead of swallow exceptions to allow withOnlineGuard to handle

Author: Zee2413

src/services/OnlineStatus.ts

@@ -1,12 +1,9 @@
 import { lookup } from 'node:dns/promises';
 import { MessageType } from 'vscode-languageserver-protocol';
 import { ClientMessage } from '../telemetry/ClientMessage';
-import { LoggerFactory } from '../telemetry/LoggerFactory';
 import { Closeable } from '../utils/Closeable';
 
 export class OnlineStatus implements Closeable {
-    private static readonly log = LoggerFactory.getLogger(OnlineStatus);
-
     private _isOnline: boolean = false;
     private notifiedOnce: boolean = false;
     private readonly timeout: NodeJS.Timeout;
@@ -22,8 +19,13 @@ export class OnlineStatus implements Closeable {
         );
     }
 
-    get isOnline() {
-        return this._isOnline;
+    public async checkNow(): Promise<boolean> {
+        try {
+            await lookup('google.com');
+            return true;
+        } catch {
+            return false;
+        }
     }
 
     private async hasInternet() {

src/utils/OnlineFeatureGuard.ts

@@ -13,13 +13,16 @@ export class OnlineFeatureGuard {
         private readonly awsCredentials: AwsCredentials,
     ) {}
 
-    check(prerequisites: OnlinePrerequisites): void {
-        if (prerequisites.requiresInternet && !this.onlineStatus.isOnline) {
-            throw createOnlineFeatureError(
-                OnlineFeatureErrorCode.NoInternet,
-                'Internet connection required for this operation. Please check your network connection.',
-                { retryable: true, requiresReauth: false },
-            );
+    async check(prerequisites: OnlinePrerequisites): Promise<void> {
+        if (prerequisites.requiresInternet) {
+            const isOnline = await this.onlineStatus.checkNow();
+            if (!isOnline) {
+                throw createOnlineFeatureError(
+                    OnlineFeatureErrorCode.NoInternet,
+                    'Internet connection required for this operation. Please check your network connection.',
+                    { retryable: true, requiresReauth: false },
+                );
+            }
         }
 
         if (prerequisites.requiresAuth && !this.awsCredentials.credentialsAvailable()) {

src/utils/OnlineFeatureWrapper.ts

@@ -19,7 +19,7 @@ export function withOnlineGuard<T extends Handler>(
     prerequisites: Partial<OnlinePrerequisites> = {},
 ): T {
     return (async (...args: any[]) => {
-        guard.check({ ...DEFAULT_PREREQUISITES, ...prerequisites });
+        await guard.check({ ...DEFAULT_PREREQUISITES, ...prerequisites });
         try {
             return await handler(...args);
         } catch (error) {

tst/unit/utils/OnlineFeatureGuard.test.ts

@@ -1,58 +1,60 @@
 import { describe, expect, it } from 'vitest';
+import { AwsCredentials } from '../../../src/auth/AwsCredentials';
+import { OnlineStatus } from '../../../src/services/OnlineStatus';
 import { OnlineFeatureErrorCode } from '../../../src/utils/OnlineFeatureError';
 import { OnlineFeatureGuard } from '../../../src/utils/OnlineFeatureGuard';
 
 describe('OnlineFeatureGuard', () => {
-    it('should pass when both internet and auth are available', () => {
-        const onlineStatus = { isOnline: true };
-        const awsCredentials = { credentialsAvailable: () => true };
-        const guard = new OnlineFeatureGuard(onlineStatus as any, awsCredentials as any);
+    it('should pass when both internet and auth are available', async () => {
+        const onlineStatus = { checkNow: () => Promise.resolve(true) } as OnlineStatus;
+        const awsCredentials = { credentialsAvailable: () => true } as AwsCredentials;
+        const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
 
-        expect(() => guard.check({ requiresInternet: true, requiresAuth: true })).not.toThrow();
+        await expect(guard.check({ requiresInternet: true, requiresAuth: true })).resolves.not.toThrow();
     });
 
-    it('should throw NoInternet when internet is required but not available', () => {
-        const onlineStatus = { isOnline: false };
-        const awsCredentials = { credentialsAvailable: () => true };
-        const guard = new OnlineFeatureGuard(onlineStatus as any, awsCredentials as any);
+    it('should throw NoInternet when internet is required but not available', async () => {
+        const onlineStatus = { checkNow: () => Promise.resolve(false) } as OnlineStatus;
+        const awsCredentials = { credentialsAvailable: () => true } as AwsCredentials;
+        const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
 
-        expect(() => guard.check({ requiresInternet: true, requiresAuth: true })).toThrow(
+        await expect(guard.check({ requiresInternet: true, requiresAuth: true })).rejects.toThrow(
             expect.objectContaining({ code: OnlineFeatureErrorCode.NoInternet }),
         );
     });
 
-    it('should throw NoAuthentication when auth is required but not available', () => {
-        const onlineStatus = { isOnline: true };
-        const awsCredentials = { credentialsAvailable: () => false };
-        const guard = new OnlineFeatureGuard(onlineStatus as any, awsCredentials as any);
+    it('should throw NoAuthentication when auth is required but not available', async () => {
+        const onlineStatus = { checkNow: () => Promise.resolve(true) } as OnlineStatus;
+        const awsCredentials = { credentialsAvailable: () => false } as AwsCredentials;
+        const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
 
-        expect(() => guard.check({ requiresInternet: true, requiresAuth: true })).toThrow(
+        await expect(guard.check({ requiresInternet: true, requiresAuth: true })).rejects.toThrow(
             expect.objectContaining({ code: OnlineFeatureErrorCode.NoAuthentication }),
         );
     });
 
-    it('should pass when internet is not required and not available', () => {
-        const onlineStatus = { isOnline: false };
-        const awsCredentials = { credentialsAvailable: () => true };
-        const guard = new OnlineFeatureGuard(onlineStatus as any, awsCredentials as any);
+    it('should pass when internet is not required and not available', async () => {
+        const onlineStatus = { checkNow: () => Promise.resolve(false) } as OnlineStatus;
+        const awsCredentials = { credentialsAvailable: () => true } as AwsCredentials;
+        const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
 
-        expect(() => guard.check({ requiresInternet: false, requiresAuth: true })).not.toThrow();
+        await expect(guard.check({ requiresInternet: false, requiresAuth: true })).resolves.not.toThrow();
     });
 
-    it('should pass when auth is not required and not available', () => {
-        const onlineStatus = { isOnline: true };
-        const awsCredentials = { credentialsAvailable: () => false };
-        const guard = new OnlineFeatureGuard(onlineStatus as any, awsCredentials as any);
+    it('should pass when auth is not required and not available', async () => {
+        const onlineStatus = { checkNow: () => Promise.resolve(true) } as OnlineStatus;
+        const awsCredentials = { credentialsAvailable: () => false } as AwsCredentials;
+        const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
 
-        expect(() => guard.check({ requiresInternet: true, requiresAuth: false })).not.toThrow();
+        await expect(guard.check({ requiresInternet: true, requiresAuth: false })).resolves.not.toThrow();
     });
 
-    it('should check internet before auth', () => {
-        const onlineStatus = { isOnline: false };
-        const awsCredentials = { credentialsAvailable: () => false };
-        const guard = new OnlineFeatureGuard(onlineStatus as any, awsCredentials as any);
+    it('should check internet before auth', async () => {
+        const onlineStatus = { checkNow: () => Promise.resolve(false) } as OnlineStatus;
+        const awsCredentials = { credentialsAvailable: () => false } as AwsCredentials;
+        const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
 
-        expect(() => guard.check({ requiresInternet: true, requiresAuth: true })).toThrow(
+        await expect(guard.check({ requiresInternet: true, requiresAuth: true })).rejects.toThrow(
             expect.objectContaining({ code: OnlineFeatureErrorCode.NoInternet }),
         );
     });