diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/proxy/cdk-requests-go-through-a-proxy-when-configured.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/proxy/cdk-requests-go-through-a-proxy-when-configured.integtest.ts index 9373037fc..76b5132d4 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/proxy/cdk-requests-go-through-a-proxy-when-configured.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/proxy/cdk-requests-go-through-a-proxy-when-configured.integtest.ts @@ -14,6 +14,9 @@ integTest('requests go through a proxy when configured', // Delete notices cache if it exists await fs.rm(path.join(cdkCacheDir, 'notices.json'), { force: true }); + // Delete connection cache if it exists + await fs.rm(path.join(cdkCacheDir, 'connection.json'), { force: true }); + await fixture.cdkDeploy('test-2', { captureStderr: true, options: [ @@ -26,7 +29,6 @@ integTest('requests go through a proxy when configured', }); const requests = await proxyServer.getSeenRequests(); - expect(requests.map(req => req.url)) .toContain('https://cli.cdk.dev-tools.aws.dev/notices.json'); diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/index.ts b/packages/@aws-cdk/toolkit-lib/lib/api/index.ts index dc221c8d7..ac2bbd005 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/index.ts @@ -10,6 +10,7 @@ export * from './garbage-collection'; export * from './hotswap'; export * from './io'; export * from './logs-monitor'; +export * from './network-detector'; export * from './notices'; export * from './plugin'; export * from './refactoring'; diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/network-detector/index.ts b/packages/@aws-cdk/toolkit-lib/lib/api/network-detector/index.ts new file mode 100644 index 000000000..0819ada2e --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/lib/api/network-detector/index.ts @@ -0,0 +1 @@ +export * from './network-detector'; diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/network-detector/network-detector.ts b/packages/@aws-cdk/toolkit-lib/lib/api/network-detector/network-detector.ts new file mode 100644 index 000000000..545cd9730 --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/lib/api/network-detector/network-detector.ts @@ -0,0 +1,98 @@ +import * as https from 'node:https'; +import type { RequestOptions } from 'node:https'; +import * as path from 'path'; +import * as fs from 'fs-extra'; +import { cdkCacheDir } from '../../util'; + +interface CachedConnectivity { + expiration: number; + hasConnectivity: boolean; +} + +const TIME_TO_LIVE_SUCCESS = 60 * 60 * 1000; // 1 hour +const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'connection.json'); + +/** + * Detects internet connectivity by making a lightweight request to the notices endpoint + */ +export class NetworkDetector { + /** + * Check if internet connectivity is available + */ + public static async hasConnectivity(agent?: https.Agent): Promise { + const cachedData = await this.load(); + const expiration = cachedData.expiration ?? 0; + + if (Date.now() > expiration) { + try { + const connected = await this.ping(agent); + const updatedData = { + expiration: Date.now() + TIME_TO_LIVE_SUCCESS, + hasConnectivity: connected, + }; + await this.save(updatedData); + return connected; + } catch { + return false; + } + } else { + return cachedData.hasConnectivity; + } + } + + // We are observing lots of timeouts when running in a massively parallel + // integration test environment, so wait for a longer timeout there. + // + // In production, have a short timeout to not hold up the user experience. + private static readonly TIMEOUT = process.env.TESTING_CDK ? 30_000 : 3_000; + private static readonly URL = 'https://cli.cdk.dev-tools.aws.dev/notices.json'; + + private static async load(): Promise { + const defaultValue = { + expiration: 0, + hasConnectivity: false, + }; + + try { + return fs.existsSync(CACHE_FILE_PATH) + ? await fs.readJSON(CACHE_FILE_PATH) as CachedConnectivity + : defaultValue; + } catch { + return defaultValue; + } + } + + private static async save(cached: CachedConnectivity): Promise { + try { + await fs.ensureFile(CACHE_FILE_PATH); + await fs.writeJSON(CACHE_FILE_PATH, cached); + } catch { + // Silently ignore cache save errors + } + } + + private static ping(agent?: https.Agent): Promise { + const options: RequestOptions = { + method: 'HEAD', + agent: agent, + timeout: this.TIMEOUT, + }; + + return new Promise((resolve) => { + const req = https.request( + NetworkDetector.URL, + options, + (res) => { + resolve(res.statusCode !== undefined && res.statusCode < 500); + }, + ); + req.on('error', () => resolve(false)); + req.on('timeout', () => { + req.destroy(); + resolve(false); + }); + + req.end(); + }); + } +} diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/notices/web-data-source.ts b/packages/@aws-cdk/toolkit-lib/lib/api/notices/web-data-source.ts index 6dcf4cc72..f4ee08d28 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/notices/web-data-source.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/notices/web-data-source.ts @@ -5,6 +5,7 @@ import type { Notice, NoticeDataSource } from './types'; import { ToolkitError } from '../../toolkit/toolkit-error'; import { formatErrorMessage, humanHttpStatusError, humanNetworkError } from '../../util'; import type { IoHelper } from '../io/private'; +import { NetworkDetector } from '../network-detector/network-detector'; /** * A data source that fetches notices from the CDK notices data source @@ -20,6 +21,7 @@ export class WebsiteNoticeDataSourceProps { * @default - Official CDK notices */ readonly url?: string | URL; + /** * The agent responsible for making the network requests. * @@ -44,6 +46,12 @@ export class WebsiteNoticeDataSource implements NoticeDataSource { } async fetch(): Promise { + // Check connectivity before attempting network request + const hasConnectivity = await NetworkDetector.hasConnectivity(this.agent); + if (!hasConnectivity) { + throw new ToolkitError('No internet connectivity detected'); + } + // We are observing lots of timeouts when running in a massively parallel // integration test environment, so wait for a longer timeout there. // @@ -66,7 +74,8 @@ export class WebsiteNoticeDataSource implements NoticeDataSource { timer.unref(); try { - req = https.get(this.url, + req = https.get( + this.url, options, res => { if (res.statusCode === 200) { @@ -92,7 +101,8 @@ export class WebsiteNoticeDataSource implements NoticeDataSource { } else { reject(new ToolkitError(`${humanHttpStatusError(res.statusCode!)} (Status code: ${res.statusCode})`)); } - }); + }, + ); req.on('error', e => { reject(ToolkitError.withCause(humanNetworkError(e), e)); }); diff --git a/packages/@aws-cdk/toolkit-lib/test/api/notices.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/notices.test.ts index e79c15e42..c7f43f833 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/notices.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/notices.test.ts @@ -5,6 +5,7 @@ import * as fs from 'fs-extra'; import * as nock from 'nock'; import { Context } from '../../lib/api/context'; import { asIoHelper } from '../../lib/api/io/private'; +import { NetworkDetector } from '../../lib/api/network-detector/network-detector'; import { Notices } from '../../lib/api/notices'; import { CachedDataSource } from '../../lib/api/notices/cached-data-source'; import { FilteredNotice, NoticesFilter } from '../../lib/api/notices/filter'; @@ -540,6 +541,24 @@ function parseTestComponent(x: string): Component { describe(WebsiteNoticeDataSource, () => { const dataSource = new WebsiteNoticeDataSource(ioHelper); + beforeEach(() => { + // Mock NetworkDetector to return true by default for existing tests + jest.spyOn(NetworkDetector, 'hasConnectivity').mockResolvedValue(true); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('throws error when no connectivity detected', async () => { + const mockHasConnectivity = jest.spyOn(NetworkDetector, 'hasConnectivity').mockResolvedValue(false); + + await expect(dataSource.fetch()).rejects.toThrow('No internet connectivity detected'); + expect(mockHasConnectivity).toHaveBeenCalledWith(undefined); + + mockHasConnectivity.mockRestore(); + }); + test('returns data when download succeeds', async () => { const result = await mockCall(200, { notices: [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE], diff --git a/packages/@aws-cdk/toolkit-lib/test/util/network-detector.test.ts b/packages/@aws-cdk/toolkit-lib/test/util/network-detector.test.ts new file mode 100644 index 000000000..fc924fd5b --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/test/util/network-detector.test.ts @@ -0,0 +1,169 @@ +import * as https from 'node:https'; +import * as fs from 'fs-extra'; +import { NetworkDetector } from '../../lib/api/network-detector/network-detector'; + +// Mock the https module +jest.mock('node:https'); +const mockHttps = https as jest.Mocked; + +// Mock fs-extra +jest.mock('fs-extra'); +const mockFs = fs as jest.Mocked; + +// Mock cdkCacheDir +jest.mock('../../lib/util', () => ({ + cdkCacheDir: jest.fn(() => '/mock/cache/dir'), +})); + +describe('NetworkDetector', () => { + let mockRequest: jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + mockRequest = jest.fn(); + mockHttps.request.mockImplementation(mockRequest); + }); + + test('returns true when server responds with success status', async () => { + const mockReq = { + on: jest.fn(), + end: jest.fn(), + destroy: jest.fn(), + }; + + mockRequest.mockImplementation((_url, _options, callback) => { + setTimeout(() => callback({ statusCode: 200 }), 0); + return mockReq; + }); + + mockFs.existsSync.mockReturnValue(false); + (mockFs.ensureFile as jest.Mock).mockResolvedValue(undefined); + (mockFs.writeJSON as jest.Mock).mockResolvedValue(undefined); + + const result = await NetworkDetector.hasConnectivity(); + expect(result).toBe(true); // Should return true for successful HTTP response + }); + + test('returns false when server responds with server error', async () => { + const mockReq = { + on: jest.fn(), + end: jest.fn(), + destroy: jest.fn(), + }; + + mockRequest.mockImplementation((_url, _options, callback) => { + setTimeout(() => callback({ statusCode: 500 }), 0); + return mockReq; + }); + + mockFs.existsSync.mockReturnValue(false); + (mockFs.ensureFile as jest.Mock).mockResolvedValue(undefined); + (mockFs.writeJSON as jest.Mock).mockResolvedValue(undefined); + + const result = await NetworkDetector.hasConnectivity(); + expect(result).toBe(false); // Should return false for server error status codes + }); + + test('returns false on network error', async () => { + const mockReq = { + on: jest.fn((event, handler) => { + if (event === 'error') { + setTimeout(() => handler(new Error('Network error')), 0); + } + }), + end: jest.fn(), + destroy: jest.fn(), + }; + + mockRequest.mockReturnValue(mockReq); + mockFs.existsSync.mockReturnValue(false); + + const result = await NetworkDetector.hasConnectivity(); + expect(result).toBe(false); // Should return false when network request fails + }); + + test('returns cached result from disk when not expired', async () => { + const cachedData = { + expiration: Date.now() + 30000, // 30 seconds in future + hasConnectivity: true, + }; + + mockFs.existsSync.mockReturnValue(true); + (mockFs.readJSON as jest.Mock).mockResolvedValue(cachedData); + + const result = await NetworkDetector.hasConnectivity(); + + expect(result).toBe(true); // Should return cached connectivity result + expect(mockRequest).not.toHaveBeenCalled(); // Should not make network request when cache is valid + }); + + test('performs ping when disk cache is expired', async () => { + const expiredData = { + expiration: Date.now() - 1000, // 1 second ago + hasConnectivity: true, + }; + + const mockReq = { + on: jest.fn(), + end: jest.fn(), + destroy: jest.fn(), + }; + + mockRequest.mockImplementation((_url, _options, callback) => { + setTimeout(() => callback({ statusCode: 200 }), 0); + return mockReq; + }); + + mockFs.existsSync.mockReturnValue(true); + (mockFs.readJSON as jest.Mock).mockResolvedValue(expiredData); + (mockFs.ensureFile as jest.Mock).mockResolvedValue(undefined); + (mockFs.writeJSON as jest.Mock).mockResolvedValue(undefined); + + const result = await NetworkDetector.hasConnectivity(); + + expect(result).toBe(true); // Should return fresh connectivity result + expect(mockRequest).toHaveBeenCalledTimes(1); // Should make network request when cache is expired + }); + + test('handles cache save errors gracefully', async () => { + const mockReq = { + on: jest.fn(), + end: jest.fn(), + destroy: jest.fn(), + }; + + mockRequest.mockImplementation((_url, _options, callback) => { + setTimeout(() => callback({ statusCode: 200 }), 0); + return mockReq; + }); + + mockFs.existsSync.mockReturnValue(false); + (mockFs.ensureFile as jest.Mock).mockRejectedValue(new Error('Disk full')); + + const result = await NetworkDetector.hasConnectivity(); + + expect(result).toBe(true); // Should still return connectivity result despite cache save failure + }); + + test('handles cache load errors gracefully', async () => { + const mockReq = { + on: jest.fn(), + end: jest.fn(), + destroy: jest.fn(), + }; + + mockRequest.mockImplementation((_url, _options, callback) => { + setTimeout(() => callback({ statusCode: 200 }), 0); + return mockReq; + }); + + mockFs.existsSync.mockReturnValue(true); + (mockFs.readJSON as jest.Mock).mockRejectedValue(new Error('Read failed')); + (mockFs.ensureFile as jest.Mock).mockResolvedValue(undefined); + (mockFs.writeJSON as jest.Mock).mockResolvedValue(undefined); + + const result = await NetworkDetector.hasConnectivity(); + + expect(result).toBe(true); // Should still return connectivity result despite cache load failure + }); +}); diff --git a/packages/aws-cdk/lib/api/network-detector.ts b/packages/aws-cdk/lib/api/network-detector.ts new file mode 100644 index 000000000..d311ee017 --- /dev/null +++ b/packages/aws-cdk/lib/api/network-detector.ts @@ -0,0 +1,2 @@ +/* eslint-disable import/no-relative-packages */ +export * from '../../../@aws-cdk/toolkit-lib/lib/api/network-detector'; diff --git a/packages/aws-cdk/lib/cli/telemetry/sink/endpoint-sink.ts b/packages/aws-cdk/lib/cli/telemetry/sink/endpoint-sink.ts index 772089a61..0c7b2d8b5 100644 --- a/packages/aws-cdk/lib/cli/telemetry/sink/endpoint-sink.ts +++ b/packages/aws-cdk/lib/cli/telemetry/sink/endpoint-sink.ts @@ -3,6 +3,7 @@ import type { Agent } from 'https'; import { request } from 'https'; import { parse, type UrlWithStringQuery } from 'url'; import { ToolkitError } from '@aws-cdk/toolkit-lib'; +import { NetworkDetector } from '../../../api/network-detector'; import { IoHelper } from '../../../api-private'; import type { IIoHost } from '../../io-host'; import type { TelemetrySchema } from '../schema'; @@ -89,6 +90,13 @@ export class EndpointTelemetrySink implements ITelemetrySink { url: UrlWithStringQuery, body: { events: TelemetrySchema[] }, ): Promise { + // Check connectivity before attempting network request + const hasConnectivity = await NetworkDetector.hasConnectivity(this.agent); + if (!hasConnectivity) { + await this.ioHelper.defaults.trace('No internet connectivity detected, skipping telemetry'); + return false; + } + try { const res = await doRequest(url, body, this.agent); diff --git a/packages/aws-cdk/test/cli/telemetry/sink/endpoint-sink.test.ts b/packages/aws-cdk/test/cli/telemetry/sink/endpoint-sink.test.ts index db7d8e182..ec024982e 100644 --- a/packages/aws-cdk/test/cli/telemetry/sink/endpoint-sink.test.ts +++ b/packages/aws-cdk/test/cli/telemetry/sink/endpoint-sink.test.ts @@ -1,5 +1,6 @@ import * as https from 'https'; import { createTestEvent } from './util'; +import { NetworkDetector } from '../../../../lib/api/network-detector'; import { IoHelper } from '../../../../lib/api-private'; import { CliIoHost } from '../../../../lib/cli/io-host'; import { EndpointTelemetrySink } from '../../../../lib/cli/telemetry/sink/endpoint-sink'; @@ -9,12 +10,22 @@ jest.mock('https', () => ({ request: jest.fn(), })); +// Mock NetworkDetector +jest.mock('../../../../lib/api/network-detector', () => ({ + NetworkDetector: { + hasConnectivity: jest.fn(), + }, +})); + describe('EndpointTelemetrySink', () => { let ioHost: CliIoHost; beforeEach(() => { jest.resetAllMocks(); + // Mock NetworkDetector to return true by default for existing tests + (NetworkDetector.hasConnectivity as jest.Mock).mockResolvedValue(true); + ioHost = CliIoHost.instance(); }); @@ -312,4 +323,20 @@ describe('EndpointTelemetrySink', () => { expect.stringContaining('Telemetry Error: POST example.com/telemetry:'), ); }); + + test('skips request when no connectivity detected', async () => { + // GIVEN + (NetworkDetector.hasConnectivity as jest.Mock).mockResolvedValue(false); + + const testEvent = createTestEvent('INVOKE', { foo: 'bar' }); + const client = new EndpointTelemetrySink({ endpoint: 'https://example.com/telemetry', ioHost }); + + // WHEN + await client.emit(testEvent); + await client.flush(); + + // THEN + expect(NetworkDetector.hasConnectivity).toHaveBeenCalledWith(undefined); + expect(https.request).not.toHaveBeenCalled(); + }); }); diff --git a/packages/aws-cdk/test/cli/telemetry/sink/funnel.test.ts b/packages/aws-cdk/test/cli/telemetry/sink/funnel.test.ts index cb3c0766f..f07c43d62 100644 --- a/packages/aws-cdk/test/cli/telemetry/sink/funnel.test.ts +++ b/packages/aws-cdk/test/cli/telemetry/sink/funnel.test.ts @@ -3,6 +3,7 @@ import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs-extra'; import { createTestEvent } from './util'; +import { NetworkDetector } from '../../../../lib/api/network-detector'; import { IoHelper } from '../../../../lib/api-private'; import { CliIoHost } from '../../../../lib/cli/io-host'; import { EndpointTelemetrySink } from '../../../../lib/cli/telemetry/sink/endpoint-sink'; @@ -14,6 +15,13 @@ jest.mock('https', () => ({ request: jest.fn(), })); +// Mock NetworkDetector +jest.mock('../../../../lib/api/network-detector', () => ({ + NetworkDetector: { + hasConnectivity: jest.fn(), + }, +})); + describe('Funnel', () => { let tempDir: string; let logFilePath: string; @@ -22,6 +30,9 @@ describe('Funnel', () => { beforeEach(() => { jest.resetAllMocks(); + // Mock NetworkDetector to return true by default for all tests + (NetworkDetector.hasConnectivity as jest.Mock).mockResolvedValue(true); + // Create a fresh temp directory for each test tempDir = path.join(os.tmpdir(), `telemetry-test-${Date.now()}-${Math.random().toString(36).substring(2, 9)}`); fs.mkdirSync(tempDir, { recursive: true }); diff --git a/packages/aws-cdk/test/commands/notices.test.ts b/packages/aws-cdk/test/commands/notices.test.ts index 4ed6ee976..e12e48974 100644 --- a/packages/aws-cdk/test/commands/notices.test.ts +++ b/packages/aws-cdk/test/commands/notices.test.ts @@ -1,3 +1,11 @@ +// Mock NetworkDetector before any imports +const mockHasConnectivity = jest.fn().mockResolvedValue(true); +jest.mock('../../../@aws-cdk/toolkit-lib/lib/api/network-detector/network-detector', () => ({ + NetworkDetector: { + hasConnectivity: mockHasConnectivity, + }, +})); + import * as nock from 'nock'; import { exec } from '../../lib/cli/cli'; @@ -21,6 +29,8 @@ const BASIC_NOTICE = { beforeEach(() => { nock.cleanAll(); jest.clearAllMocks(); + // Ensure NetworkDetector always returns true for connectivity + mockHasConnectivity.mockResolvedValue(true); }); describe('cdk notices', () => {