Skip to content

Commit 295ddc6

Browse files
kaizenccgithub-actions
andauthored
feat(toolkit-lib): network detector (#926)
this PR implements a connectivity detector that performs a HEAD request to the notices endpoint, with a timeout of 1 hour. before making any subsequent network calls, (including GET notices and PUT telemetry) we check to see if we have already determined the network to be disconnected. in those cases, we skip subsequent calls. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license --------- Signed-off-by: github-actions <github-actions@github.com> Co-authored-by: github-actions <github-actions@github.com>
1 parent 1847bfe commit 295ddc6

File tree

12 files changed

+361
-3
lines changed

12 files changed

+361
-3
lines changed

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/proxy/cdk-requests-go-through-a-proxy-when-configured.integtest.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ integTest('requests go through a proxy when configured',
1414
// Delete notices cache if it exists
1515
await fs.rm(path.join(cdkCacheDir, 'notices.json'), { force: true });
1616

17+
// Delete connection cache if it exists
18+
await fs.rm(path.join(cdkCacheDir, 'connection.json'), { force: true });
19+
1720
await fixture.cdkDeploy('test-2', {
1821
captureStderr: true,
1922
options: [
@@ -26,7 +29,6 @@ integTest('requests go through a proxy when configured',
2629
});
2730

2831
const requests = await proxyServer.getSeenRequests();
29-
3032
expect(requests.map(req => req.url))
3133
.toContain('https://cli.cdk.dev-tools.aws.dev/notices.json');
3234

packages/@aws-cdk/toolkit-lib/lib/api/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export * from './garbage-collection';
1010
export * from './hotswap';
1111
export * from './io';
1212
export * from './logs-monitor';
13+
export * from './network-detector';
1314
export * from './notices';
1415
export * from './plugin';
1516
export * from './refactoring';
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export * from './network-detector';
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import * as https from 'node:https';
2+
import type { RequestOptions } from 'node:https';
3+
import * as path from 'path';
4+
import * as fs from 'fs-extra';
5+
import { cdkCacheDir } from '../../util';
6+
7+
interface CachedConnectivity {
8+
expiration: number;
9+
hasConnectivity: boolean;
10+
}
11+
12+
const TIME_TO_LIVE_SUCCESS = 60 * 60 * 1000; // 1 hour
13+
const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'connection.json');
14+
15+
/**
16+
* Detects internet connectivity by making a lightweight request to the notices endpoint
17+
*/
18+
export class NetworkDetector {
19+
/**
20+
* Check if internet connectivity is available
21+
*/
22+
public static async hasConnectivity(agent?: https.Agent): Promise<boolean> {
23+
const cachedData = await this.load();
24+
const expiration = cachedData.expiration ?? 0;
25+
26+
if (Date.now() > expiration) {
27+
try {
28+
const connected = await this.ping(agent);
29+
const updatedData = {
30+
expiration: Date.now() + TIME_TO_LIVE_SUCCESS,
31+
hasConnectivity: connected,
32+
};
33+
await this.save(updatedData);
34+
return connected;
35+
} catch {
36+
return false;
37+
}
38+
} else {
39+
return cachedData.hasConnectivity;
40+
}
41+
}
42+
43+
// We are observing lots of timeouts when running in a massively parallel
44+
// integration test environment, so wait for a longer timeout there.
45+
//
46+
// In production, have a short timeout to not hold up the user experience.
47+
private static readonly TIMEOUT = process.env.TESTING_CDK ? 30_000 : 3_000;
48+
private static readonly URL = 'https://cli.cdk.dev-tools.aws.dev/notices.json';
49+
50+
private static async load(): Promise<CachedConnectivity> {
51+
const defaultValue = {
52+
expiration: 0,
53+
hasConnectivity: false,
54+
};
55+
56+
try {
57+
return fs.existsSync(CACHE_FILE_PATH)
58+
? await fs.readJSON(CACHE_FILE_PATH) as CachedConnectivity
59+
: defaultValue;
60+
} catch {
61+
return defaultValue;
62+
}
63+
}
64+
65+
private static async save(cached: CachedConnectivity): Promise<void> {
66+
try {
67+
await fs.ensureFile(CACHE_FILE_PATH);
68+
await fs.writeJSON(CACHE_FILE_PATH, cached);
69+
} catch {
70+
// Silently ignore cache save errors
71+
}
72+
}
73+
74+
private static ping(agent?: https.Agent): Promise<boolean> {
75+
const options: RequestOptions = {
76+
method: 'HEAD',
77+
agent: agent,
78+
timeout: this.TIMEOUT,
79+
};
80+
81+
return new Promise((resolve) => {
82+
const req = https.request(
83+
NetworkDetector.URL,
84+
options,
85+
(res) => {
86+
resolve(res.statusCode !== undefined && res.statusCode < 500);
87+
},
88+
);
89+
req.on('error', () => resolve(false));
90+
req.on('timeout', () => {
91+
req.destroy();
92+
resolve(false);
93+
});
94+
95+
req.end();
96+
});
97+
}
98+
}

packages/@aws-cdk/toolkit-lib/lib/api/notices/web-data-source.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { Notice, NoticeDataSource } from './types';
55
import { ToolkitError } from '../../toolkit/toolkit-error';
66
import { formatErrorMessage, humanHttpStatusError, humanNetworkError } from '../../util';
77
import type { IoHelper } from '../io/private';
8+
import { NetworkDetector } from '../network-detector/network-detector';
89

910
/**
1011
* A data source that fetches notices from the CDK notices data source
@@ -20,6 +21,7 @@ export class WebsiteNoticeDataSourceProps {
2021
* @default - Official CDK notices
2122
*/
2223
readonly url?: string | URL;
24+
2325
/**
2426
* The agent responsible for making the network requests.
2527
*
@@ -44,6 +46,12 @@ export class WebsiteNoticeDataSource implements NoticeDataSource {
4446
}
4547

4648
async fetch(): Promise<Notice[]> {
49+
// Check connectivity before attempting network request
50+
const hasConnectivity = await NetworkDetector.hasConnectivity(this.agent);
51+
if (!hasConnectivity) {
52+
throw new ToolkitError('No internet connectivity detected');
53+
}
54+
4755
// We are observing lots of timeouts when running in a massively parallel
4856
// integration test environment, so wait for a longer timeout there.
4957
//
@@ -66,7 +74,8 @@ export class WebsiteNoticeDataSource implements NoticeDataSource {
6674
timer.unref();
6775

6876
try {
69-
req = https.get(this.url,
77+
req = https.get(
78+
this.url,
7079
options,
7180
res => {
7281
if (res.statusCode === 200) {
@@ -92,7 +101,8 @@ export class WebsiteNoticeDataSource implements NoticeDataSource {
92101
} else {
93102
reject(new ToolkitError(`${humanHttpStatusError(res.statusCode!)} (Status code: ${res.statusCode})`));
94103
}
95-
});
104+
},
105+
);
96106
req.on('error', e => {
97107
reject(ToolkitError.withCause(humanNetworkError(e), e));
98108
});

packages/@aws-cdk/toolkit-lib/test/api/notices.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import * as fs from 'fs-extra';
55
import * as nock from 'nock';
66
import { Context } from '../../lib/api/context';
77
import { asIoHelper } from '../../lib/api/io/private';
8+
import { NetworkDetector } from '../../lib/api/network-detector/network-detector';
89
import { Notices } from '../../lib/api/notices';
910
import { CachedDataSource } from '../../lib/api/notices/cached-data-source';
1011
import { FilteredNotice, NoticesFilter } from '../../lib/api/notices/filter';
@@ -540,6 +541,24 @@ function parseTestComponent(x: string): Component {
540541
describe(WebsiteNoticeDataSource, () => {
541542
const dataSource = new WebsiteNoticeDataSource(ioHelper);
542543

544+
beforeEach(() => {
545+
// Mock NetworkDetector to return true by default for existing tests
546+
jest.spyOn(NetworkDetector, 'hasConnectivity').mockResolvedValue(true);
547+
});
548+
549+
afterEach(() => {
550+
jest.restoreAllMocks();
551+
});
552+
553+
test('throws error when no connectivity detected', async () => {
554+
const mockHasConnectivity = jest.spyOn(NetworkDetector, 'hasConnectivity').mockResolvedValue(false);
555+
556+
await expect(dataSource.fetch()).rejects.toThrow('No internet connectivity detected');
557+
expect(mockHasConnectivity).toHaveBeenCalledWith(undefined);
558+
559+
mockHasConnectivity.mockRestore();
560+
});
561+
543562
test('returns data when download succeeds', async () => {
544563
const result = await mockCall(200, {
545564
notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE],
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
import * as https from 'node:https';
2+
import * as fs from 'fs-extra';
3+
import { NetworkDetector } from '../../lib/api/network-detector/network-detector';
4+
5+
// Mock the https module
6+
jest.mock('node:https');
7+
const mockHttps = https as jest.Mocked<typeof https>;
8+
9+
// Mock fs-extra
10+
jest.mock('fs-extra');
11+
const mockFs = fs as jest.Mocked<typeof fs>;
12+
13+
// Mock cdkCacheDir
14+
jest.mock('../../lib/util', () => ({
15+
cdkCacheDir: jest.fn(() => '/mock/cache/dir'),
16+
}));
17+
18+
describe('NetworkDetector', () => {
19+
let mockRequest: jest.Mock;
20+
21+
beforeEach(() => {
22+
jest.clearAllMocks();
23+
mockRequest = jest.fn();
24+
mockHttps.request.mockImplementation(mockRequest);
25+
});
26+
27+
test('returns true when server responds with success status', async () => {
28+
const mockReq = {
29+
on: jest.fn(),
30+
end: jest.fn(),
31+
destroy: jest.fn(),
32+
};
33+
34+
mockRequest.mockImplementation((_url, _options, callback) => {
35+
setTimeout(() => callback({ statusCode: 200 }), 0);
36+
return mockReq;
37+
});
38+
39+
mockFs.existsSync.mockReturnValue(false);
40+
(mockFs.ensureFile as jest.Mock).mockResolvedValue(undefined);
41+
(mockFs.writeJSON as jest.Mock).mockResolvedValue(undefined);
42+
43+
const result = await NetworkDetector.hasConnectivity();
44+
expect(result).toBe(true); // Should return true for successful HTTP response
45+
});
46+
47+
test('returns false when server responds with server error', async () => {
48+
const mockReq = {
49+
on: jest.fn(),
50+
end: jest.fn(),
51+
destroy: jest.fn(),
52+
};
53+
54+
mockRequest.mockImplementation((_url, _options, callback) => {
55+
setTimeout(() => callback({ statusCode: 500 }), 0);
56+
return mockReq;
57+
});
58+
59+
mockFs.existsSync.mockReturnValue(false);
60+
(mockFs.ensureFile as jest.Mock).mockResolvedValue(undefined);
61+
(mockFs.writeJSON as jest.Mock).mockResolvedValue(undefined);
62+
63+
const result = await NetworkDetector.hasConnectivity();
64+
expect(result).toBe(false); // Should return false for server error status codes
65+
});
66+
67+
test('returns false on network error', async () => {
68+
const mockReq = {
69+
on: jest.fn((event, handler) => {
70+
if (event === 'error') {
71+
setTimeout(() => handler(new Error('Network error')), 0);
72+
}
73+
}),
74+
end: jest.fn(),
75+
destroy: jest.fn(),
76+
};
77+
78+
mockRequest.mockReturnValue(mockReq);
79+
mockFs.existsSync.mockReturnValue(false);
80+
81+
const result = await NetworkDetector.hasConnectivity();
82+
expect(result).toBe(false); // Should return false when network request fails
83+
});
84+
85+
test('returns cached result from disk when not expired', async () => {
86+
const cachedData = {
87+
expiration: Date.now() + 30000, // 30 seconds in future
88+
hasConnectivity: true,
89+
};
90+
91+
mockFs.existsSync.mockReturnValue(true);
92+
(mockFs.readJSON as jest.Mock).mockResolvedValue(cachedData);
93+
94+
const result = await NetworkDetector.hasConnectivity();
95+
96+
expect(result).toBe(true); // Should return cached connectivity result
97+
expect(mockRequest).not.toHaveBeenCalled(); // Should not make network request when cache is valid
98+
});
99+
100+
test('performs ping when disk cache is expired', async () => {
101+
const expiredData = {
102+
expiration: Date.now() - 1000, // 1 second ago
103+
hasConnectivity: true,
104+
};
105+
106+
const mockReq = {
107+
on: jest.fn(),
108+
end: jest.fn(),
109+
destroy: jest.fn(),
110+
};
111+
112+
mockRequest.mockImplementation((_url, _options, callback) => {
113+
setTimeout(() => callback({ statusCode: 200 }), 0);
114+
return mockReq;
115+
});
116+
117+
mockFs.existsSync.mockReturnValue(true);
118+
(mockFs.readJSON as jest.Mock).mockResolvedValue(expiredData);
119+
(mockFs.ensureFile as jest.Mock).mockResolvedValue(undefined);
120+
(mockFs.writeJSON as jest.Mock).mockResolvedValue(undefined);
121+
122+
const result = await NetworkDetector.hasConnectivity();
123+
124+
expect(result).toBe(true); // Should return fresh connectivity result
125+
expect(mockRequest).toHaveBeenCalledTimes(1); // Should make network request when cache is expired
126+
});
127+
128+
test('handles cache save errors gracefully', async () => {
129+
const mockReq = {
130+
on: jest.fn(),
131+
end: jest.fn(),
132+
destroy: jest.fn(),
133+
};
134+
135+
mockRequest.mockImplementation((_url, _options, callback) => {
136+
setTimeout(() => callback({ statusCode: 200 }), 0);
137+
return mockReq;
138+
});
139+
140+
mockFs.existsSync.mockReturnValue(false);
141+
(mockFs.ensureFile as jest.Mock).mockRejectedValue(new Error('Disk full'));
142+
143+
const result = await NetworkDetector.hasConnectivity();
144+
145+
expect(result).toBe(true); // Should still return connectivity result despite cache save failure
146+
});
147+
148+
test('handles cache load errors gracefully', async () => {
149+
const mockReq = {
150+
on: jest.fn(),
151+
end: jest.fn(),
152+
destroy: jest.fn(),
153+
};
154+
155+
mockRequest.mockImplementation((_url, _options, callback) => {
156+
setTimeout(() => callback({ statusCode: 200 }), 0);
157+
return mockReq;
158+
});
159+
160+
mockFs.existsSync.mockReturnValue(true);
161+
(mockFs.readJSON as jest.Mock).mockRejectedValue(new Error('Read failed'));
162+
(mockFs.ensureFile as jest.Mock).mockResolvedValue(undefined);
163+
(mockFs.writeJSON as jest.Mock).mockResolvedValue(undefined);
164+
165+
const result = await NetworkDetector.hasConnectivity();
166+
167+
expect(result).toBe(true); // Should still return connectivity result despite cache load failure
168+
});
169+
});
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
/* eslint-disable import/no-relative-packages */
2+
export * from '../../../@aws-cdk/toolkit-lib/lib/api/network-detector';

0 commit comments

Comments
 (0)