Skip to content

Commit ac509dc

Browse files
committed
feat(toolkit-lib): network detector
1 parent 9b286a2 commit ac509dc

File tree

6 files changed

+72
-11
lines changed

6 files changed

+72
-11
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import * as https from 'node:https';
44
import type { Notice, NoticeDataSource } from './types';
55
import { ToolkitError } from '../../toolkit/toolkit-error';
66
import { formatErrorMessage, humanHttpStatusError, humanNetworkError } from '../../util';
7+
import { NetworkDetector } from '../../util/network-detector';
78
import type { IoHelper } from '../io/private';
89

910
/**
@@ -44,6 +45,12 @@ export class WebsiteNoticeDataSource implements NoticeDataSource {
4445
}
4546

4647
async fetch(): Promise<Notice[]> {
48+
// Check connectivity before attempting network request
49+
const hasConnectivity = await NetworkDetector.hasConnectivity(this.agent);
50+
if (!hasConnectivity) {
51+
throw new ToolkitError('No internet connectivity detected');
52+
}
53+
4754
// We are observing lots of timeouts when running in a massively parallel
4855
// integration test environment, so wait for a longer timeout there.
4956
//

packages/@aws-cdk/toolkit-lib/lib/util/network-detector.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,12 @@ import { request } from 'https';
55
* Detects internet connectivity by making a lightweight request to the notices endpoint
66
*/
77
export class NetworkDetector {
8-
private static readonly CACHE_DURATION_MS = 30_000; // 30 seconds
9-
private static readonly TIMEOUT_MS = 500;
10-
11-
private static cachedResult: boolean | undefined;
12-
private static cacheExpiry: number = 0;
13-
148
/**
159
* Check if internet connectivity is available
1610
*/
1711
public static async hasConnectivity(agent?: Agent): Promise<boolean> {
1812
const now = Date.now();
19-
13+
2014
// Return cached result if still valid
2115
if (this.cachedResult !== undefined && now < this.cacheExpiry) {
2216
return this.cachedResult;
@@ -34,6 +28,12 @@ export class NetworkDetector {
3428
}
3529
}
3630

31+
private static readonly CACHE_DURATION_MS = 30_000; // 30 seconds
32+
private static readonly TIMEOUT_MS = 500;
33+
34+
private static cachedResult: boolean | undefined;
35+
private static cacheExpiry: number = 0;
36+
3737
private static ping(agent?: Agent): Promise<boolean> {
3838
return new Promise((resolve) => {
3939
const req = request({

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { FilteredNotice, NoticesFilter } from '../../lib/api/notices/filter';
1111
import type { BootstrappedEnvironment, Component, Notice } from '../../lib/api/notices/types';
1212
import { WebsiteNoticeDataSource } from '../../lib/api/notices/web-data-source';
1313
import { Settings } from '../../lib/api/settings';
14+
import { NetworkDetector } from '../../lib/util/network-detector';
1415
import { TestIoHost } from '../_helpers';
1516

1617
const BASIC_BOOTSTRAP_NOTICE = {
@@ -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],

packages/@aws-cdk/toolkit-lib/test/util/network-detector.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe('NetworkDetector', () => {
2525
destroy: jest.fn(),
2626
};
2727

28-
mockRequest.mockImplementation((_, callback) => {
28+
mockRequest.mockImplementation((_options, callback) => {
2929
callback({ statusCode: 200 });
3030
return mockReq;
3131
});
@@ -41,7 +41,7 @@ describe('NetworkDetector', () => {
4141
destroy: jest.fn(),
4242
};
4343

44-
mockRequest.mockImplementation((_, callback) => {
44+
mockRequest.mockImplementation((_options, callback) => {
4545
callback({ statusCode: 500 });
4646
return mockReq;
4747
});
@@ -91,7 +91,7 @@ describe('NetworkDetector', () => {
9191
destroy: jest.fn(),
9292
};
9393

94-
mockRequest.mockImplementation((_, callback) => {
94+
mockRequest.mockImplementation((_options, callback) => {
9595
callback({ statusCode: 200 });
9696
return mockReq;
9797
});

packages/aws-cdk/lib/cli/telemetry/sink/endpoint-sink.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { IncomingMessage } from 'http';
22
import type { Agent } from 'https';
33
import { request } from 'https';
44
import { parse, type UrlWithStringQuery } from 'url';
5-
import { ToolkitError } from '@aws-cdk/toolkit-lib';
5+
import { ToolkitError, NetworkDetector } from '@aws-cdk/toolkit-lib';
66
import { IoHelper } from '../../../api-private';
77
import type { IIoHost } from '../../io-host';
88
import type { TelemetrySchema } from '../schema';
@@ -94,6 +94,13 @@ export class EndpointTelemetrySink implements ITelemetrySink {
9494
url: UrlWithStringQuery,
9595
body: { events: TelemetrySchema[] },
9696
): Promise<boolean> {
97+
// Check connectivity before attempting network request
98+
const hasConnectivity = await NetworkDetector.hasConnectivity(this.agent);
99+
if (!hasConnectivity) {
100+
await this.ioHelper.defaults.trace('No internet connectivity detected, skipping telemetry');
101+
return false;
102+
}
103+
97104
try {
98105
const res = await doRequest(url, body, this.agent);
99106

packages/aws-cdk/test/cli/telemetry/sink/endpoint-sink.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,30 @@ import { createTestEvent } from './util';
33
import { IoHelper } from '../../../../lib/api-private';
44
import { CliIoHost } from '../../../../lib/cli/io-host';
55
import { EndpointTelemetrySink } from '../../../../lib/cli/telemetry/sink/endpoint-sink';
6+
import { NetworkDetector } from '@aws-cdk/toolkit-lib/lib/util/network-detector';
67

78
// Mock the https module
89
jest.mock('https', () => ({
910
request: jest.fn(),
1011
}));
1112

13+
// Mock NetworkDetector
14+
jest.mock('@aws-cdk/toolkit-lib', () => ({
15+
...jest.requireActual('@aws-cdk/toolkit-lib'),
16+
NetworkDetector: {
17+
hasConnectivity: jest.fn(),
18+
},
19+
}));
20+
1221
describe('EndpointTelemetrySink', () => {
1322
let ioHost: CliIoHost;
1423

1524
beforeEach(() => {
1625
jest.resetAllMocks();
1726

27+
// Mock NetworkDetector to return true by default for existing tests
28+
(NetworkDetector.hasConnectivity as jest.Mock).mockResolvedValue(true);
29+
1830
ioHost = CliIoHost.instance();
1931
});
2032

@@ -312,4 +324,20 @@ describe('EndpointTelemetrySink', () => {
312324
expect.stringContaining('Telemetry Error: POST example.com/telemetry:'),
313325
);
314326
});
327+
328+
test('skips request when no connectivity detected', async () => {
329+
// GIVEN
330+
(NetworkDetector.hasConnectivity as jest.Mock).mockResolvedValue(false);
331+
332+
const testEvent = createTestEvent('INVOKE', { foo: 'bar' });
333+
const client = new EndpointTelemetrySink({ endpoint: 'https://example.com/telemetry', ioHost });
334+
335+
// WHEN
336+
await client.emit(testEvent);
337+
await client.flush();
338+
339+
// THEN
340+
expect(NetworkDetector.hasConnectivity).toHaveBeenCalledWith(undefined);
341+
expect(https.request).not.toHaveBeenCalled();
342+
});
315343
});

0 commit comments

Comments
 (0)