make throw

Author: mrgrain

.projenrc.ts

@@ -1008,6 +1008,7 @@ const cli = configureProject(
       'jest-environment-node',
       'jest-mock',
       'madge',
+      'nock@13',
       'sinon',
       'ts-mock-imports',
       'xml-js',

packages/@aws-cdk/toolkit-lib/lib/api/notices/cached-data-soure.ts

@@ -1,6 +1,7 @@
 import * as fs from 'fs-extra';
 import type { Notice, NoticeDataSource } from './types';
 import type { IoDefaultMessages } from '../io/private';
+import { ToolkitError } from '../toolkit-error';
 
 interface CachedNotices {
   expiration: number;
@@ -25,28 +26,32 @@ export class CachedDataSource implements NoticeDataSource {
     const expiration = cachedData.expiration ?? 0;
 
     if (Date.now() > expiration || this.skipCache) {
-      const freshData = await this.fetchInner();
-      await this.save(freshData);
-      return freshData.notices;
+      let updatedData: CachedNotices = cachedData;
+
+      try {
+        updatedData = await this.fetchInner();
+      } catch (e) {
+        this.ioMessages.debug(`Could not refresh notices: ${e}`);
+        updatedData = {
+          expiration: Date.now() + TIME_TO_LIVE_ERROR,
+          notices: [],
+        };
+        throw ToolkitError.withCause('Failed to load CDK notices. Please try again later.', e);
+      } finally {
+        await this.save(updatedData);
+      }
+      return updatedData.notices;
     } else {
       this.ioMessages.debug(`Reading cached notices from ${this.fileName}`);
       return data;
     }
   }
 
   private async fetchInner(): Promise<CachedNotices> {
-    try {
-      return {
-        expiration: Date.now() + TIME_TO_LIVE_SUCCESS,
-        notices: await this.dataSource.fetch(),
-      };
-    } catch (e) {
-      this.ioMessages.debug(`Could not refresh notices: ${e}`);
-      return {
-        expiration: Date.now() + TIME_TO_LIVE_ERROR,
-        notices: [],
-      };
-    }
+    return {
+      expiration: Date.now() + TIME_TO_LIVE_SUCCESS,
+      notices: await this.dataSource.fetch(),
+    };
   }
 
   private async load(): Promise<CachedNotices> {

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

@@ -1,13 +1,13 @@
 import type { ClientRequest } from 'http';
 import type { RequestOptions } from 'https';
 import * as https from 'node:https';
-import { formatErrorMessage } from '../../util';
 import type { SdkHttpOptions } from '../aws-auth';
 import { ProxyAgentProvider } from '../aws-auth/private';
 import type { IoHelper } from '../io/private';
 import { IO } from '../io/private';
 import { ToolkitError } from '../toolkit-error';
 import type { Notice, NoticeDataSource } from './types';
+import { formatErrorMessage, humanHttpStatusError, humanNetworkError } from '../../util';
 
 export class WebsiteNoticeDataSource implements NoticeDataSource {
   private readonly options: SdkHttpOptions;
@@ -48,23 +48,25 @@ export class WebsiteNoticeDataSource implements NoticeDataSource {
                 try {
                   const data = JSON.parse(rawData).notices as Notice[];
                   if (!data) {
-                    throw new ToolkitError("'notices' key is missing");
+                    throw new ToolkitError("'notices' key is missing from received data");
                   }
                   resolve(data ?? []);
                 } catch (e: any) {
-                  reject(new ToolkitError(`Failed to parse notices: ${formatErrorMessage(e)}`));
+                  reject(ToolkitError.withCause(formatErrorMessage(e), e));
                 }
               });
               res.on('error', e => {
-                reject(new ToolkitError(`Failed to fetch notices: ${formatErrorMessage(e)}`));
+                reject(ToolkitError.withCause(formatErrorMessage(e), e));
               });
             } else {
-              reject(new ToolkitError(`Failed to fetch notices. Status code: ${res.statusCode}`));
+              reject(new ToolkitError(`${humanHttpStatusError(res.statusCode!)} (Status code: ${res.statusCode})`));
             }
           });
-        req.on('error', reject);
+        req.on('error', e => {
+          reject(ToolkitError.withCause(humanNetworkError(e), e));
+        });
       } catch (e: any) {
-        reject(new ToolkitError(`HTTPS 'get' call threw an error: ${formatErrorMessage(e)}`));
+        reject(ToolkitError.withCause(formatErrorMessage(e), e));
       }
     });
 

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

@@ -7,6 +7,7 @@ export * from './content-hash';
 export * from './directories';
 export * from './format-error';
 export * from './json';
+export * from './net';
 export * from './objects';
 export * from './parallel';
 export * from './package-info';

packages/@aws-cdk/toolkit-lib/lib/util/net.ts

@@ -0,0 +1,56 @@
+/**
+ * Get a human-readable error message for a network error
+ * @param error The network error object
+ */
+export function humanNetworkError(error: NodeJS.ErrnoException): string {
+  switch (error.code) {
+    case 'ENOTFOUND':
+      return `Cannot reach the server. Please check your internet connection or the URL (${(error as any).hostname}).`;
+    case 'ECONNREFUSED':
+      return `Connection refused. The server at ${(error as any).address}:${(error as any).port} is not accepting connections.`;
+    case 'ECONNRESET':
+      return 'Connection was suddenly closed by the server. Please try again later.';
+    case 'ETIMEDOUT':
+      return 'Connection timed out. The server took too long to respond.';
+    case 'CERT_HAS_EXPIRED':
+      return 'The SSL certificate of the server has expired. This could be a security risk.';
+    case 'UNABLE_TO_VERIFY_LEAF_SIGNATURE':
+    case 'CERT_SIGNATURE_FAILURE':
+    case 'ERR_TLS_CERT_ALTNAME_INVALID':
+      return 'SSL certificate validation failed. This could indicate a security issue or a misconfigured server.';
+    default:
+      return `Network error: ${error.message || error.code || 'Unknown error'}`;
+  }
+}
+
+/**
+ * Get a human-readable error message for a HTTP status code
+ */
+export function humanHttpStatusError(statusCode: number): string {
+  switch (statusCode) {
+    case 400:
+      return 'Bad request - the server could not understand the request';
+    case 401:
+      return 'Unauthorized - authentication is required';
+    case 403:
+      return 'Forbidden - you do not have permission to access this resource';
+    case 404:
+      return 'Not found - the requested resource does not exist';
+    case 408:
+      return 'Request timeout - the server timed out waiting for the request';
+    case 429:
+      return 'Too many requests - you have sent too many requests in a given amount of time';
+    case 500:
+      return 'Internal server error - something went wrong on the server';
+    case 502:
+      return 'Bad gateway - the server received an invalid response from an upstream server';
+    case 503:
+      return 'Service unavailable - the server is temporarily unable to handle the request';
+    case 504:
+      return 'Gateway timeout - the server did not receive a timely response from an upstream server';
+    default:
+      return statusCode >= 500
+        ? 'Server error - something went wrong on the server'
+        : 'Client error - something went wrong with the request';
+  }
+}

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

@@ -605,7 +605,7 @@ describe(WebsiteNoticeDataSource, () => {
     await expect(result).rejects.toThrow(/timed out/);
   });
 
-  test('returns empty array when the request takes too long to finish', async () => {
+  test('returns appropriate error when the request takes too long to finish', async () => {
     nock('https://cli.cdk.dev-tools.aws.dev')
       .get('/notices.json')
       .delayBody(3500)
@@ -808,15 +808,17 @@ describe(Notices, () => {
       ioHost.expectMessage({ containing: 'There are 0 unacknowledged notice(s).' });
     });
 
-    test('doesnt throw', async () => {
+    test('re-throws error from data source', async () => {
       const notices = Notices.create({ ioHost, context: new Context(), cliVersion });
-      await notices.refresh({
+      const res = notices.refresh({
         dataSource: {
           fetch: async () => {
-            throw new Error('Should not fail refresh');
+            throw new Error('Should fail refresh');
           },
         },
       });
+
+      await expect(res).rejects.toThrow('Should fail refresh');
     });
 
     test('filters out acknowledged notices by default', async () => {

packages/aws-cdk/.projen/deps.json

@@ -9,7 +9,7 @@ export function prettyPrintError(error: unknown, debug = false) {
   if (err.cause) {
     const cause = ensureError(err.cause);
     console.error(chalk.yellow(cause.message));
-    printTrace(err, debug);
+    printTrace(cause, debug);
   }
 
   printTrace(err, debug);

packages/aws-cdk/lib/cli/pretty-print-error.ts

@@ -0,0 +1,51 @@
+import * as nock from 'nock';
+import { exec } from '../../lib/cli/cli';
+
+const BASIC_NOTICE = {
+  title: 'Toggling off auto_delete_objects for Bucket empties the bucket',
+  issueNumber: 16603,
+  overview:
+      'If a stack is deployed with an S3 bucket with auto_delete_objects=True, and then re-deployed with auto_delete_objects=False, all the objects in the bucket will be deleted.',
+  components: [
+    {
+      name: 'cli',
+      version: '<=1.126.0',
+    },
+  ],
+  schemaVersion: '1',
+};
+
+beforeEach(() => {
+  jest.clearAllMocks();
+});
+
+describe('cdk notices', () => {
+  test('will fail on dns error', async () => {
+    // GIVEN
+    nock('https://cli.cdk.dev-tools.aws.dev')
+      .get('/notices.json')
+      .replyWithError('DNS resolution failed');
+
+    // WHEN
+    const res = exec(['notices']);
+
+    // THEN
+    await expect(res).rejects.toThrow('DNS resolution failed');
+  });
+
+  test('will fail on timeout', async () => {
+    // GIVEN
+    nock('https://cli.cdk.dev-tools.aws.dev')
+      .get('/notices.json')
+      .delayConnection(3500)
+      .reply(200, {
+        notices: [BASIC_NOTICE],
+      });
+
+    // WHEN
+    const res = exec(['notices']);
+
+    // THEN
+    await expect(res).rejects.toThrow('Request timed out');
+  });
+});