update ctrl+c behavior to always print guidance and not prompt (#2160)

* update ctrl+c behavior to always print guidance and not prompt * pr feedback * Update .changeset/bright-jokes-draw.md Co-authored-by: Kamil Sobol <[email protected]> --------- Co-authored-by: Kamil Sobol <[email protected]>
aws-amplify · Oct 30, 2024 · c3c3057 · c3c3057
1 parent 9a5d8e3
commit c3c3057
Show file tree

Hide file tree

Showing 8 changed files with 18 additions and 167 deletions.
diff --git a/.changeset/bright-jokes-draw.md b/.changeset/bright-jokes-draw.md
@@ -0,0 +1,6 @@
+---
+'@aws-amplify/cli-core': minor
+'@aws-amplify/backend-cli': minor
+---
+
+update ctrl+c behavior to always print guidance to delete and exit with no prompt
diff --git a/packages/cli-core/src/package-manager-controller/package_manager_controller_base.ts b/packages/cli-core/src/package-manager-controller/package_manager_controller_base.ts
@@ -137,6 +137,7 @@ export abstract class PackageManagerControllerBase
   /**
    * allowsSignalPropagation - Determines if the package manager allows the process
    * signals such as SIGINT to be propagated to the underlying node process.
+   * @deprecated
    */
   allowsSignalPropagation = () => true;
 

diff --git a/packages/cli-core/src/package-manager-controller/pnpm_package_manager_controller.ts b/packages/cli-core/src/package-manager-controller/pnpm_package_manager_controller.ts
@@ -32,10 +32,4 @@ export class PnpmPackageManagerController extends PackageManagerControllerBase {
       existsSync
     );
   }
-
-  /**
-   * Pnpm doesn't handle the node process gracefully during the SIGINT life cycle.
-   * See: https://github.com/pnpm/pnpm/issues/7374
-   */
-  allowsSignalPropagation = () => false;
 }
diff --git a/packages/cli-core/src/package-manager-controller/yarn_classic_package_manager_controller.ts b/packages/cli-core/src/package-manager-controller/yarn_classic_package_manager_controller.ts
@@ -37,12 +37,6 @@ export class YarnClassicPackageManagerController extends PackageManagerControlle
     await this.addTypescript(targetDir);
     await super.initializeTsConfig(targetDir);
   };
-  /**
-   *
-   * Yarn doesn't respect the SIGINT life cycle and exits immediately leaving
-   * the node process hanging. See: https://github.com/yarnpkg/yarn/issues/8895
-   */
-  allowsSignalPropagation = () => false;
 
   private addTypescript = async (targetDir: string) => {
     await this.executeWithDebugLogger(

diff --git a/packages/cli/src/commands/sandbox/sandbox-delete/sandbox_delete_command.test.ts b/packages/cli/src/commands/sandbox/sandbox-delete/sandbox_delete_command.test.ts
@@ -1,10 +1,5 @@
 import { beforeEach, describe, it, mock } from 'node:test';
-import {
-  AmplifyPrompter,
-  PackageManagerControllerFactory,
-  format,
-  printer,
-} from '@aws-amplify/cli-core';
+import { AmplifyPrompter, format, printer } from '@aws-amplify/cli-core';
 import yargs, { CommandModule } from 'yargs';
 import { TestCommandRunner } from '../../../test-utils/command_runner.js';
 import assert from 'node:assert';
@@ -50,8 +45,7 @@ void describe('sandbox delete command', () => {
       sandboxFactory,
       [sandboxDeleteCommand, createSandboxSecretCommand()],
       clientConfigGeneratorAdapterMock,
-      commandMiddleware,
-      new PackageManagerControllerFactory().getPackageManagerController()
+      commandMiddleware
     );
     const parser = yargs().command(sandboxCommand as unknown as CommandModule);
     commandRunner = new TestCommandRunner(parser);

diff --git a/packages/cli/src/commands/sandbox/sandbox_command.test.ts b/packages/cli/src/commands/sandbox/sandbox_command.test.ts
@@ -8,7 +8,7 @@ import {
   TestCommandError,
   TestCommandRunner,
 } from '../../test-utils/command_runner.js';
-import { AmplifyPrompter, format, printer } from '@aws-amplify/cli-core';
+import { format, printer } from '@aws-amplify/cli-core';
 import { EventHandler, SandboxCommand } from './sandbox_command.js';
 import { createSandboxCommand } from './sandbox_command_factory.js';
 import { SandboxDeleteCommand } from './sandbox-delete/sandbox_delete_command.js';
@@ -20,7 +20,6 @@ import {
 import { createSandboxSecretCommand } from './sandbox-secret/sandbox_secret_command_factory.js';
 import { ClientConfigGeneratorAdapter } from '../../client-config/client_config_generator_adapter.js';
 import { CommandMiddleware } from '../../command_middleware.js';
-import { PackageManagerController } from '@aws-amplify/plugin-types';
 import { AmplifyError } from '@aws-amplify/platform-core';
 
 mock.method(fsp, 'mkdir', () => Promise.resolve());
@@ -54,11 +53,6 @@ void describe('sandbox command', () => {
   );
   const sandboxProfile = 'test-sandbox';
 
-  const allowsSignalPropagationMock = mock.fn(() => true);
-  const packageManagerControllerMock = {
-    allowsSignalPropagation: allowsSignalPropagationMock,
-  } as unknown as PackageManagerController;
-
   beforeEach(async () => {
     const sandboxFactory = new SandboxSingletonFactory(
       () =>
@@ -80,7 +74,6 @@ void describe('sandbox command', () => {
       [sandboxDeleteCommand, createSandboxSecretCommand()],
       clientConfigGeneratorAdapterMock,
       commandMiddleware,
-      packageManagerControllerMock,
       () => ({
         successfulDeployment: [clientConfigGenerationMock],
         successfulDeletion: [clientConfigDeletionMock],
@@ -189,118 +182,7 @@ void describe('sandbox command', () => {
     );
   });
 
-  void it('asks to delete the sandbox environment when users send ctrl-C and say yes to delete', async (contextual) => {
-    // Mock process and extract the sigint handler after calling the sandbox command
-    const processSignal = contextual.mock.method(process, 'on', () => {
-      /* no op */
-    });
-    const sandboxStartMock = contextual.mock.method(
-      sandbox,
-      'start',
-      async () => Promise.resolve()
-    );
-
-    const sandboxDeleteMock = contextual.mock.method(sandbox, 'delete', () =>
-      Promise.resolve()
-    );
-
-    // User said yes to delete
-    contextual.mock.method(AmplifyPrompter, 'yesOrNo', () =>
-      Promise.resolve(true)
-    );
-
-    await commandRunner.runCommand('sandbox');
-
-    // Similar to the later 0ms timeout. Without this tests in github action are failing
-    // but working locally
-    await new Promise((resolve) => setTimeout(resolve, 0));
-    const sigIntHandlerFn = processSignal.mock.calls[0].arguments[1];
-    if (sigIntHandlerFn) sigIntHandlerFn();
-
-    // I can't find any open node:test or yargs issues that would explain why this is necessary
-    // but for some reason the mock call count does not update without this 0ms wait
-    await new Promise((resolve) => setTimeout(resolve, 0));
-    assert.equal(sandboxStartMock.mock.callCount(), 1);
-    assert.equal(sandboxDeleteMock.mock.callCount(), 1);
-  });
-
-  void it('asks to delete the sandbox environment when users send ctrl-C and say yes to delete with profile', async (contextual) => {
-    // Mock process and extract the sigint handler after calling the sandbox command
-    const processSignal = contextual.mock.method(process, 'on', () => {
-      /* no op */
-    });
-    const sandboxStartMock = contextual.mock.method(
-      sandbox,
-      'start',
-      async () => Promise.resolve()
-    );
-
-    const sandboxDeleteMock = contextual.mock.method(sandbox, 'delete', () =>
-      Promise.resolve()
-    );
-
-    // User said yes to delete
-    contextual.mock.method(AmplifyPrompter, 'yesOrNo', () =>
-      Promise.resolve(true)
-    );
-
-    const profile = 'test_profile';
-    await commandRunner.runCommand(`sandbox --profile ${profile}`);
-
-    // Similar to the later 0ms timeout. Without this tests in github action are failing
-    // but working locally
-    await new Promise((resolve) => setTimeout(resolve, 0));
-    const sigIntHandlerFn = processSignal.mock.calls[0].arguments[1];
-    if (sigIntHandlerFn) sigIntHandlerFn();
-
-    // I can't find any open node:test or yargs issues that would explain why this is necessary
-    // but for some reason the mock call count does not update without this 0ms wait
-    await new Promise((resolve) => setTimeout(resolve, 0));
-    assert.equal(sandboxStartMock.mock.callCount(), 1);
-    assert.equal(sandboxDeleteMock.mock.callCount(), 1);
-    assert.deepStrictEqual(sandboxDeleteMock.mock.calls[0].arguments[0], {
-      identifier: undefined,
-      profile,
-    });
-  });
-
-  void it('asks to delete the sandbox environment when users send ctrl-C and say no to delete', async (contextual) => {
-    // Mock process and extract the sigint handler after calling the sandbox command
-    const processSignal = contextual.mock.method(process, 'on', () => {
-      /* no op */
-    });
-    const sandboxStartMock = contextual.mock.method(
-      sandbox,
-      'start',
-      async () => Promise.resolve()
-    );
-
-    const sandboxDeleteMock = contextual.mock.method(
-      sandbox,
-      'delete',
-      async () => Promise.resolve()
-    );
-
-    // User said no to delete
-    contextual.mock.method(AmplifyPrompter, 'yesOrNo', () =>
-      Promise.resolve(false)
-    );
-
-    await commandRunner.runCommand('sandbox');
-
-    // Similar to the previous test's 0ms timeout. Without this tests in github action are failing
-    // but working locally
-    await new Promise((resolve) => setTimeout(resolve, 0));
-    const sigIntHandlerFn = processSignal.mock.calls[0].arguments[1];
-    if (sigIntHandlerFn) sigIntHandlerFn();
-
-    assert.equal(sandboxStartMock.mock.callCount(), 1);
-    assert.equal(sandboxDeleteMock.mock.callCount(), 0);
-  });
-
-  void it('Does not prompt for deleting the sandbox if package manager does not allow signal propagation', async (contextual) => {
-    allowsSignalPropagationMock.mock.mockImplementationOnce(() => false);
-
+  void it('Prints stopping sandbox and instructions to delete sandbox when users send ctrl+c', async (contextual) => {
     // Mock process and extract the sigint handler after calling the sandbox command
     const processSignal = contextual.mock.method(process, 'on', () => {
       /* no op */
@@ -371,7 +253,6 @@ void describe('sandbox command', () => {
       [],
       clientConfigGeneratorAdapterMock,
       commandMiddleware,
-      packageManagerControllerMock,
       undefined
     );
     const parser = yargs().command(sandboxCommand as unknown as CommandModule);

diff --git a/packages/cli/src/commands/sandbox/sandbox_command.ts b/packages/cli/src/commands/sandbox/sandbox_command.ts
@@ -1,7 +1,7 @@
 import { ArgumentsCamelCase, Argv, CommandModule } from 'yargs';
 import fs from 'fs';
 import fsp from 'fs/promises';
-import { AmplifyPrompter, format, printer } from '@aws-amplify/cli-core';
+import { format, printer } from '@aws-amplify/cli-core';
 import {
   SandboxFunctionStreamingOptions,
   SandboxSingletonFactory,
@@ -20,7 +20,6 @@ import { ClientConfigGeneratorAdapter } from '../../client-config/client_config_
 import { CommandMiddleware } from '../../command_middleware.js';
 import { SandboxCommandGlobalOptions } from './option_types.js';
 import { ArgumentsKebabCase } from '../../kebab_case.js';
-import { PackageManagerController } from '@aws-amplify/plugin-types';
 import { AmplifyUserError } from '@aws-amplify/platform-core';
 
 export type SandboxCommandOptionsKebabCase = ArgumentsKebabCase<
@@ -81,7 +80,6 @@ export class SandboxCommand
     private readonly sandboxSubCommands: CommandModule[],
     private clientConfigGeneratorAdapter: ClientConfigGeneratorAdapter,
     private commandMiddleware: CommandMiddleware,
-    private readonly packageManagerController: PackageManagerController,
     private readonly sandboxEventHandlerCreator?: SandboxEventHandlerCreator
   ) {
     this.command = 'sandbox';
@@ -276,23 +274,11 @@ export class SandboxCommand
   };
 
   sigIntHandler = async () => {
-    if (!this.packageManagerController.allowsSignalPropagation()) {
-      printer.print(
-        `Stopping the sandbox process. To delete the sandbox, run ${format.normalizeAmpxCommand(
-          'sandbox delete'
-        )}`
-      );
-      return;
-    }
-    const answer = await AmplifyPrompter.yesOrNo({
-      message:
-        'Would you like to delete all the resources in your sandbox environment (This cannot be undone)?',
-      defaultValue: false,
-    });
-    if (answer)
-      await (
-        await this.sandboxFactory.getInstance()
-      ).delete({ identifier: this.sandboxIdentifier, profile: this.profile });
+    printer.print(
+      `Stopping the sandbox process. To delete the sandbox, run ${format.normalizeAmpxCommand(
+        'sandbox delete'
+      )}`
+    );
   };
 
   private validateDirectory = async (option: string, dir: string) => {

diff --git a/packages/cli/src/commands/sandbox/sandbox_command_factory.ts b/packages/cli/src/commands/sandbox/sandbox_command_factory.ts
@@ -16,11 +16,7 @@ import {
 } from '@aws-amplify/platform-core';
 import { SandboxEventHandlerFactory } from './sandbox_event_handler_factory.js';
 import { CommandMiddleware } from '../../command_middleware.js';
-import {
-  PackageManagerControllerFactory,
-  format,
-  printer,
-} from '@aws-amplify/cli-core';
+import { format, printer } from '@aws-amplify/cli-core';
 import { S3Client } from '@aws-sdk/client-s3';
 import { AmplifyClient } from '@aws-sdk/client-amplify';
 import { CloudFormationClient } from '@aws-sdk/client-cloudformation';
@@ -70,7 +66,6 @@ export const createSandboxCommand = (): CommandModule<
     [new SandboxDeleteCommand(sandboxFactory), createSandboxSecretCommand()],
     clientConfigGeneratorAdapter,
     commandMiddleWare,
-    new PackageManagerControllerFactory().getPackageManagerController(),
     eventHandlerFactory.getSandboxEventHandlers
   );
 };