Skip to content

Commit f767a5a

Browse files
Zee2413Zee2413
committed
remove internet check from online guard
1 parent 2816b9e commit f767a5a

File tree

6 files changed

+14
-117
lines changed

6 files changed

+14
-117
lines changed

‎src/server/CfnExternal.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ export class CfnExternal implements Configurables, Closeable {
6565

6666
this.onlineStatus = overrides.onlineStatus ?? new OnlineStatus(core.clientMessage);
6767
this.featureFlags = overrides.featureFlags ?? new FeatureFlagProvider(getFromGitHub);
68-
this.onlineFeatureGuard =
69-
overrides.onlineFeatureGuard ?? new OnlineFeatureGuard(this.onlineStatus, core.awsCredentials);
68+
this.onlineFeatureGuard = overrides.onlineFeatureGuard ?? new OnlineFeatureGuard(core.awsCredentials);
7069
}
7170

7271
configurables(): Configurable[] {

‎src/services/OnlineStatus.ts‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,6 @@ export class OnlineStatus implements Closeable {
1919
);
2020
}
2121

22-
public async checkNow(): Promise<boolean> {
23-
try {
24-
await lookup('google.com');
25-
return true;
26-
} catch {
27-
return false;
28-
}
29-
}
30-
3122
private async hasInternet() {
3223
try {
3324
await lookup('google.com');

‎src/utils/OnlineFeatureGuard.ts‎

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,11 @@
11
import { AwsCredentials } from '../auth/AwsCredentials';
2-
import { OnlineStatus } from '../services/OnlineStatus';
32
import { createOnlineFeatureError, OnlineFeatureErrorCode } from './OnlineFeatureError';
43

5-
export interface OnlinePrerequisites {
6-
requiresInternet: boolean;
7-
requiresAuth: boolean;
8-
}
9-
104
export class OnlineFeatureGuard {
11-
constructor(
12-
private readonly onlineStatus: OnlineStatus,
13-
private readonly awsCredentials: AwsCredentials,
14-
) {}
15-
16-
async check(prerequisites: OnlinePrerequisites): Promise<void> {
17-
if (prerequisites.requiresInternet) {
18-
const isOnline = await this.onlineStatus.checkNow();
19-
if (!isOnline) {
20-
throw createOnlineFeatureError(
21-
OnlineFeatureErrorCode.NoInternet,
22-
'Internet connection required for this operation. Please check your network connection.',
23-
{ retryable: true, requiresReauth: false },
24-
);
25-
}
26-
}
5+
constructor(private readonly awsCredentials: AwsCredentials) {}
276

28-
if (prerequisites.requiresAuth && !this.awsCredentials.credentialsAvailable()) {
7+
check(): void {
8+
if (!this.awsCredentials.credentialsAvailable()) {
299
throw createOnlineFeatureError(
3010
OnlineFeatureErrorCode.NoAuthentication,
3111
'AWS credentials required for this operation. Please configure your AWS credentials.',

‎src/utils/OnlineFeatureWrapper.ts‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,19 @@
11
import { LoggerFactory } from '../telemetry/LoggerFactory';
22
import { mapAwsErrorToLspError } from './AwsErrorMapper';
3-
import { OnlineFeatureGuard, OnlinePrerequisites } from './OnlineFeatureGuard';
4-
import { toString } from './String';
5-
6-
const DEFAULT_PREREQUISITES: OnlinePrerequisites = {
7-
requiresInternet: true,
8-
requiresAuth: true,
9-
};
3+
import { OnlineFeatureGuard } from './OnlineFeatureGuard';
104

115
const log = LoggerFactory.getLogger('withOnlineGuard');
126

137
/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-argument */
148
type Handler = (...args: any[]) => any;
159

16-
export function withOnlineGuard<T extends Handler>(
17-
guard: OnlineFeatureGuard,
18-
handler: T,
19-
prerequisites: Partial<OnlinePrerequisites> = {},
20-
): T {
10+
export function withOnlineGuard<T extends Handler>(guard: OnlineFeatureGuard, handler: T): T {
2111
return (async (...args: any[]) => {
22-
await guard.check({ ...DEFAULT_PREREQUISITES, ...prerequisites });
12+
guard.check();
2313
try {
2414
return await handler(...args);
2515
} catch (error) {
26-
log.error(error, `Online feature guard check failed with prerequisites: ${toString(prerequisites)}`);
16+
log.error(error, `Online feature guard check failed`);
2717
throw mapAwsErrorToLspError(error);
2818
}
2919
}) as T;

‎tst/unit/utils/OnlineFeatureGuard.test.ts‎

Lines changed: 6 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,20 @@
11
import { describe, expect, it } from 'vitest';
22
import { AwsCredentials } from '../../../src/auth/AwsCredentials';
3-
import { OnlineStatus } from '../../../src/services/OnlineStatus';
43
import { OnlineFeatureErrorCode } from '../../../src/utils/OnlineFeatureError';
54
import { OnlineFeatureGuard } from '../../../src/utils/OnlineFeatureGuard';
65

76
describe('OnlineFeatureGuard', () => {
8-
it('should pass when both internet and auth are available', async () => {
9-
const onlineStatus = { checkNow: () => Promise.resolve(true) } as OnlineStatus;
7+
it('should pass when auth is available', () => {
108
const awsCredentials = { credentialsAvailable: () => true } as AwsCredentials;
11-
const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
9+
const guard = new OnlineFeatureGuard(awsCredentials);
1210

13-
await expect(guard.check({ requiresInternet: true, requiresAuth: true })).resolves.not.toThrow();
11+
expect(() => guard.check()).not.toThrow();
1412
});
1513

16-
it('should throw NoInternet when internet is required but not available', async () => {
17-
const onlineStatus = { checkNow: () => Promise.resolve(false) } as OnlineStatus;
18-
const awsCredentials = { credentialsAvailable: () => true } as AwsCredentials;
19-
const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
20-
21-
await expect(guard.check({ requiresInternet: true, requiresAuth: true })).rejects.toThrow(
22-
expect.objectContaining({ code: OnlineFeatureErrorCode.NoInternet }),
23-
);
24-
});
25-
26-
it('should throw NoAuthentication when auth is required but not available', async () => {
27-
const onlineStatus = { checkNow: () => Promise.resolve(true) } as OnlineStatus;
28-
const awsCredentials = { credentialsAvailable: () => false } as AwsCredentials;
29-
const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
30-
31-
await expect(guard.check({ requiresInternet: true, requiresAuth: true })).rejects.toThrow(
32-
expect.objectContaining({ code: OnlineFeatureErrorCode.NoAuthentication }),
33-
);
34-
});
35-
36-
it('should pass when internet is not required and not available', async () => {
37-
const onlineStatus = { checkNow: () => Promise.resolve(false) } as OnlineStatus;
38-
const awsCredentials = { credentialsAvailable: () => true } as AwsCredentials;
39-
const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
40-
41-
await expect(guard.check({ requiresInternet: false, requiresAuth: true })).resolves.not.toThrow();
42-
});
43-
44-
it('should pass when auth is not required and not available', async () => {
45-
const onlineStatus = { checkNow: () => Promise.resolve(true) } as OnlineStatus;
46-
const awsCredentials = { credentialsAvailable: () => false } as AwsCredentials;
47-
const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
48-
49-
await expect(guard.check({ requiresInternet: true, requiresAuth: false })).resolves.not.toThrow();
50-
});
51-
52-
it('should check internet before auth', async () => {
53-
const onlineStatus = { checkNow: () => Promise.resolve(false) } as OnlineStatus;
14+
it('should throw NoAuthentication when auth is required but not available', () => {
5415
const awsCredentials = { credentialsAvailable: () => false } as AwsCredentials;
55-
const guard = new OnlineFeatureGuard(onlineStatus, awsCredentials);
16+
const guard = new OnlineFeatureGuard(awsCredentials);
5617

57-
await expect(guard.check({ requiresInternet: true, requiresAuth: true })).rejects.toThrow(
58-
expect.objectContaining({ code: OnlineFeatureErrorCode.NoInternet }),
59-
);
18+
expect(() => guard.check()).toThrow(expect.objectContaining({ code: OnlineFeatureErrorCode.NoAuthentication }));
6019
});
6120
});

‎tst/unit/utils/OnlineFeatureWrapper.test.ts‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,6 @@ import { OnlineFeatureErrorCode } from '../../../src/utils/OnlineFeatureError';
44
import { withOnlineGuard } from '../../../src/utils/OnlineFeatureWrapper';
55

66
describe('withOnlineGuard', () => {
7-
it('should check prerequisites before calling handler', async () => {
8-
const guard = { check: vi.fn() };
9-
const handler = vi.fn().mockResolvedValue('result');
10-
11-
const wrapped = withOnlineGuard(guard as any, handler as any);
12-
const result = await wrapped('params', 'token');
13-
14-
expect(guard.check).toHaveBeenCalledWith({ requiresInternet: true, requiresAuth: true });
15-
expect(handler).toHaveBeenCalledWith('params', 'token');
16-
expect(result).toBe('result');
17-
});
18-
19-
it('should allow overriding prerequisites', async () => {
20-
const guard = { check: vi.fn() };
21-
const handler = vi.fn().mockResolvedValue('result');
22-
23-
const wrapped = withOnlineGuard(guard as any, handler as any, { requiresAuth: false });
24-
await wrapped('params', 'token');
25-
26-
expect(guard.check).toHaveBeenCalledWith({ requiresInternet: true, requiresAuth: false });
27-
});
28-
297
it('should throw when guard check fails', async () => {
308
const error = new ResponseError(OnlineFeatureErrorCode.NoInternet, 'No internet');
319
const guard = {

0 commit comments

Comments
 (0)