do not stream function logs if stack does not exist (#2315)

rtpascual · web-flow · commit 3cf073865a15 · 2024-12-11T08:52:08.000-08:00
* start streaming function logs if deployment successful

* update detection of BackendOutputClientErrors

* change to not stream only if stack does not exist

* update changeset

* remove eslint package from changeset
diff --git a/.changeset/few-candles-wink.md b/.changeset/few-candles-wink.md
@@ -0,0 +1,8 @@
+---
+'@aws-amplify/deployed-backend-client': minor
+'@aws-amplify/model-generator': patch
+'@aws-amplify/client-config': patch
+'@aws-amplify/backend-cli': patch
+---
+
+update detection of BackendOutputClientErrors
diff --git a/.changeset/great-mugs-warn.md b/.changeset/great-mugs-warn.md
@@ -0,0 +1,5 @@
+---
+'@aws-amplify/sandbox': patch
+---
+
+do not stream function logs if stack does not exist
diff --git a/packages/cli/src/commands/generate/forms/generate_forms_command.ts b/packages/cli/src/commands/generate/forms/generate_forms_command.ts
@@ -83,7 +83,7 @@ export class GenerateFormsCommand
       output = await backendOutputClient.getOutput(backendIdentifier);
     } catch (error) {
       if (
-        error instanceof BackendOutputClientError &&
+        BackendOutputClientError.isBackendOutputClientError(error) &&
         error.code === BackendOutputClientErrorType.DEPLOYMENT_IN_PROGRESS
       ) {
         throw new AmplifyUserError(
@@ -96,7 +96,7 @@ export class GenerateFormsCommand
         );
       }
       if (
-        error instanceof BackendOutputClientError &&
+        BackendOutputClientError.isBackendOutputClientError(error) &&
         error.code === BackendOutputClientErrorType.NO_STACK_FOUND
       ) {
         throw new AmplifyUserError(
@@ -110,7 +110,7 @@ export class GenerateFormsCommand
         );
       }
       if (
-        error instanceof BackendOutputClientError &&
+        BackendOutputClientError.isBackendOutputClientError(error) &&
         error.code === BackendOutputClientErrorType.CREDENTIALS_ERROR
       ) {
         throw new AmplifyUserError(
@@ -125,7 +125,7 @@ export class GenerateFormsCommand
         );
       }
       if (
-        error instanceof BackendOutputClientError &&
+        BackendOutputClientError.isBackendOutputClientError(error) &&
         error.code === BackendOutputClientErrorType.ACCESS_DENIED
       ) {
         throw new AmplifyUserError(
diff --git a/packages/client-config/src/unified_client_config_generator.ts b/packages/client-config/src/unified_client_config_generator.ts
@@ -40,7 +40,7 @@ export class UnifiedClientConfigGenerator implements ClientConfigGenerator {
       output = await this.fetchOutput();
     } catch (error) {
       if (
-        error instanceof BackendOutputClientError &&
+        BackendOutputClientError.isBackendOutputClientError(error) &&
         error.code === BackendOutputClientErrorType.DEPLOYMENT_IN_PROGRESS
       ) {
         throw new AmplifyUserError(
@@ -53,7 +53,7 @@ export class UnifiedClientConfigGenerator implements ClientConfigGenerator {
         );
       }
       if (
-        error instanceof BackendOutputClientError &&
+        BackendOutputClientError.isBackendOutputClientError(error) &&
         error.code === BackendOutputClientErrorType.NO_STACK_FOUND
       ) {
         throw new AmplifyUserError(
@@ -67,7 +67,7 @@ export class UnifiedClientConfigGenerator implements ClientConfigGenerator {
         );
       }
       if (
-        error instanceof BackendOutputClientError &&
+        BackendOutputClientError.isBackendOutputClientError(error) &&
         error.code === BackendOutputClientErrorType.METADATA_RETRIEVAL_ERROR
       ) {
         throw new AmplifyUserError(
@@ -81,7 +81,7 @@ export class UnifiedClientConfigGenerator implements ClientConfigGenerator {
         );
       }
       if (
-        error instanceof BackendOutputClientError &&
+        BackendOutputClientError.isBackendOutputClientError(error) &&
         error.code === BackendOutputClientErrorType.CREDENTIALS_ERROR
       ) {
         throw new AmplifyUserError(
@@ -96,7 +96,7 @@ export class UnifiedClientConfigGenerator implements ClientConfigGenerator {
         );
       }
       if (
-        error instanceof BackendOutputClientError &&
+        BackendOutputClientError.isBackendOutputClientError(error) &&
         error.code === BackendOutputClientErrorType.ACCESS_DENIED
       ) {
         throw new AmplifyUserError(
diff --git a/packages/deployed-backend-client/API.md b/packages/deployed-backend-client/API.md
@@ -89,6 +89,7 @@ export class BackendOutputClientError extends Error {
     constructor(code: BackendOutputClientErrorType, message: string, options?: ErrorOptions);
     // (undocumented)
     code: BackendOutputClientErrorType;
+    static isBackendOutputClientError: (error: unknown) => error is BackendOutputClientError;
 }
 
 // @public (undocumented)
diff --git a/packages/deployed-backend-client/src/backend_output_client_factory.ts b/packages/deployed-backend-client/src/backend_output_client_factory.ts
@@ -31,6 +31,30 @@ export class BackendOutputClientError extends Error {
     super(message, options);
     this.code = code;
   }
+
+  /**
+   * This function is a type predicate for BackendOutputClientError.
+   * See https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates.
+   *
+   * Checks if error is an BackendOutputClientError by inspecting if required properties are set.
+   * This is recommended instead of instanceof operator.
+   * The instance of operator does not work as expected if BackendOutputClientError class is loaded
+   * from multiple sources, for example when package manager decides to not de-duplicate dependencies.
+   * See https://github.com/nodejs/node/issues/17943.
+   */
+  static isBackendOutputClientError = (
+    error: unknown
+  ): error is BackendOutputClientError => {
+    return (
+      error instanceof Error &&
+      'code' in error &&
+      typeof error.code === 'string' &&
+      (Object.values(BackendOutputClientErrorType) as unknown[]).includes(
+        error.code
+      ) &&
+      typeof error.message === 'string'
+    );
+  };
 }
 
 /**
diff --git a/packages/eslint-rules/src/index.ts b/packages/eslint-rules/src/index.ts
@@ -3,10 +3,13 @@ import { amplifyErrorNameRule } from './rules/amplify_error_name.js';
 import { preferAmplifyErrorsRule } from './rules/prefer_amplify_errors.js';
 import { noAmplifyErrors } from './rules/no_amplify_errors.js';
 import { amplifyErrorNoInstanceOf } from './rules/amplify_error_no_instance_of';
+import { backendOutputClientErrorNoInstanceOf } from './rules/backent_output_client_error_no_instance_of.js';
 
 export const rules: Record<string, unknown> = {
   'amplify-error-name': amplifyErrorNameRule,
   'amplify-error-no-instanceof': amplifyErrorNoInstanceOf,
+  'backend-output-client-error-no-instanceof':
+    backendOutputClientErrorNoInstanceOf,
   'no-empty-catch': noEmptyCatchRule,
   'prefer-amplify-errors': preferAmplifyErrorsRule,
   'no-amplify-errors': noAmplifyErrors,
@@ -18,6 +21,8 @@ export const configs = {
     rules: {
       'amplify-backend-rules/amplify-error-name': 'error',
       'amplify-backend-rules/amplify-error-no-instanceof': 'error',
+      'amplify-backend-rules/backend-output-client-error-no-instanceof':
+        'error',
       'amplify-backend-rules/no-empty-catch': 'error',
       'amplify-backend-rules/prefer-amplify-errors': 'off',
       'amplify-backend-rules/no-amplify-errors': 'off',
diff --git a/packages/eslint-rules/src/rules/backend_output_client_error_no_instance_of.test.ts b/packages/eslint-rules/src/rules/backend_output_client_error_no_instance_of.test.ts
@@ -0,0 +1,32 @@
+import * as nodeTest from 'node:test';
+import { RuleTester } from '@typescript-eslint/rule-tester';
+import { backendOutputClientErrorNoInstanceOf } from './backent_output_client_error_no_instance_of';
+
+RuleTester.afterAll = nodeTest.after;
+// See https://typescript-eslint.io/packages/rule-tester/#with-specific-frameworks
+// Node test runner methods return promises which are not relevant in the context of testing.
+// We do ignore them in other places with void keyword.
+// eslint-disable-next-line @typescript-eslint/no-misused-promises
+RuleTester.it = nodeTest.it;
+// eslint-disable-next-line @typescript-eslint/no-misused-promises
+RuleTester.describe = nodeTest.describe;
+
+const ruleTester = new RuleTester();
+
+ruleTester.run(
+  'backend-output-client-error-no-instanceof',
+  backendOutputClientErrorNoInstanceOf,
+  {
+    valid: ['e instanceof Error'],
+    invalid: [
+      {
+        code: 'e instanceof BackendOutputClientError',
+        errors: [
+          {
+            messageId: 'noInstanceOfWithBackendOutputClientError',
+          },
+        ],
+      },
+    ],
+  }
+);
diff --git a/packages/eslint-rules/src/rules/backent_output_client_error_no_instance_of.ts b/packages/eslint-rules/src/rules/backent_output_client_error_no_instance_of.ts
@@ -0,0 +1,43 @@
+import { ESLintUtils } from '@typescript-eslint/utils';
+
+/**
+ * This rule flags empty catch blocks. Even if they contain comments.
+ *
+ * This rule differs from built in https://github.com/eslint/eslint/blob/main/lib/rules/no-empty.js
+ * in such a way that it uses typescript-eslint and typescript AST
+ * which does not include comments as statements in catch clause body block.
+ */
+export const backendOutputClientErrorNoInstanceOf =
+  ESLintUtils.RuleCreator.withoutDocs({
+    create(context) {
+      return {
+        // This naming comes from @typescript-eslint/utils types.
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        BinaryExpression(node) {
+          if (
+            node.operator === 'instanceof' &&
+            node.right.type === 'Identifier' &&
+            node.right.name === 'BackendOutputClientError'
+          ) {
+            context.report({
+              messageId: 'noInstanceOfWithBackendOutputClientError',
+              node,
+            });
+          }
+        },
+      };
+    },
+    meta: {
+      docs: {
+        description:
+          'Instanceof operator must not be used with BackendOutputClientError.',
+      },
+      messages: {
+        noInstanceOfWithBackendOutputClientError:
+          'Do not use instanceof with BackendOutputClientError. Use BackendOutputClientError.isBackendOutputClientError instead.',
+      },
+      type: 'problem',
+      schema: [],
+    },
+    defaultOptions: [],
+  });
diff --git a/packages/model-generator/src/get_backend_output_with_error_handling.ts b/packages/model-generator/src/get_backend_output_with_error_handling.ts
@@ -17,7 +17,7 @@ export const getBackendOutputWithErrorHandling = async (
     return await backendOutputClient.getOutput(backendIdentifier);
   } catch (error) {
     if (
-      error instanceof BackendOutputClientError &&
+      BackendOutputClientError.isBackendOutputClientError(error) &&
       error.code === BackendOutputClientErrorType.DEPLOYMENT_IN_PROGRESS
     ) {
       throw new AmplifyUserError(
@@ -30,7 +30,7 @@ export const getBackendOutputWithErrorHandling = async (
       );
     }
     if (
-      error instanceof BackendOutputClientError &&
+      BackendOutputClientError.isBackendOutputClientError(error) &&
       error.code === BackendOutputClientErrorType.NO_STACK_FOUND
     ) {
       throw new AmplifyUserError(
@@ -44,7 +44,7 @@ export const getBackendOutputWithErrorHandling = async (
       );
     }
     if (
-      error instanceof BackendOutputClientError &&
+      BackendOutputClientError.isBackendOutputClientError(error) &&
       error.code === BackendOutputClientErrorType.CREDENTIALS_ERROR
     ) {
       throw new AmplifyUserError(
@@ -58,7 +58,7 @@ export const getBackendOutputWithErrorHandling = async (
       );
     }
     if (
-      error instanceof BackendOutputClientError &&
+      BackendOutputClientError.isBackendOutputClientError(error) &&
       error.code === BackendOutputClientErrorType.ACCESS_DENIED
     ) {
       throw new AmplifyUserError(
diff --git a/packages/sandbox/src/lambda_function_log_streamer.test.ts b/packages/sandbox/src/lambda_function_log_streamer.test.ts
@@ -1,7 +1,11 @@
 import { beforeEach, describe, it, mock } from 'node:test';
 import { LambdaFunctionLogStreamer } from './lambda_function_log_streamer.js';
 import assert from 'node:assert';
-import { BackendOutputClient } from '@aws-amplify/deployed-backend-client';
+import {
+  BackendOutputClient,
+  BackendOutputClientError,
+  BackendOutputClientErrorType,
+} from '@aws-amplify/deployed-backend-client';
 
 import { CloudWatchLogsClient } from '@aws-sdk/client-cloudwatch-logs';
 import {
@@ -147,6 +151,23 @@ void describe('LambdaFunctionLogStreamer', () => {
     assert.strictEqual(lambdaClientSendMock.mock.callCount(), 0);
   });
 
+  void it('return early if backend output client throws with stack does not exist', async () => {
+    backendOutputClientMock.getOutput.mock.mockImplementationOnce(() => {
+      return Promise.reject(
+        new BackendOutputClientError(
+          BackendOutputClientErrorType.NO_STACK_FOUND,
+          'Stack with id test-stack does not exist'
+        )
+      );
+    });
+    await classUnderTest.startStreamingLogs(testSandboxBackendId, {
+      enabled: true,
+    });
+
+    // No lambda calls to retrieve tags
+    assert.strictEqual(lambdaClientSendMock.mock.callCount(), 0);
+  });
+
   void it('calls logs monitor with all the customer defined functions and conversation handlers if no function name filter is provided', async () => {
     await classUnderTest.startStreamingLogs(testSandboxBackendId, {
       enabled: true,
diff --git a/packages/sandbox/src/lambda_function_log_streamer.ts b/packages/sandbox/src/lambda_function_log_streamer.ts
@@ -1,5 +1,9 @@
 import { LogLevel, Printer } from '@aws-amplify/cli-core';
-import { BackendOutputClient } from '@aws-amplify/deployed-backend-client';
+import {
+  BackendOutputClient,
+  BackendOutputClientError,
+  BackendOutputClientErrorType,
+} from '@aws-amplify/deployed-backend-client';
 import { TagName } from '@aws-amplify/platform-core';
 import { BackendIdentifier, BackendOutput } from '@aws-amplify/plugin-types';
 
@@ -37,8 +41,21 @@ export class LambdaFunctionLogStreamer {
       return;
     }
 
-    const backendOutput: BackendOutput =
-      await this.backendOutputClient.getOutput(sandboxBackendId);
+    let backendOutput: BackendOutput = {};
+    try {
+      backendOutput = await this.backendOutputClient.getOutput(
+        sandboxBackendId
+      );
+    } catch (error) {
+      // If stack does not exist, we do not want to go further to start streaming logs
+      if (
+        BackendOutputClientError.isBackendOutputClientError(error) &&
+        error.code === BackendOutputClientErrorType.NO_STACK_FOUND
+      ) {
+        this.enabled = false;
+        return;
+      }
+    }
 
     const definedFunctionsPayload =
       backendOutput['AWS::Amplify::Function']?.payload.definedFunctions;

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'@aws-amplify/sandbox': patch
 +---
++
 +do not stream function logs if stack does not exist
Original file line number	Diff line number	Diff line change
`@@ -89,6 +89,7 @@ export class BackendOutputClientError extends Error {`
`89`	`89`	`constructor(code: BackendOutputClientErrorType, message: string, options?: ErrorOptions);`
`90`	`90`	`// (undocumented)`
`91`	`91`	`code: BackendOutputClientErrorType;`
	`92`	`+ static isBackendOutputClientError: (error: unknown) => error is BackendOutputClientError;`
`92`	`93`	`}`
`93`	`94`
`94`	`95`	`// @public (undocumented)`