diff --git a/.changeset/empty-cameras-smile.md b/.changeset/empty-cameras-smile.md new file mode 100644 index 00000000000..a53a3dc1b0e --- /dev/null +++ b/.changeset/empty-cameras-smile.md @@ -0,0 +1,7 @@ +--- +'@aws-amplify/platform-core': patch +'@aws-amplify/sandbox': patch +'@aws-amplify/backend-cli': patch +--- + +refactor top level cli error handling diff --git a/package-lock.json b/package-lock.json index 3ac4b6a83e0..eeaaa9d0af1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -27795,7 +27795,7 @@ }, "packages/backend-data": { "name": "@aws-amplify/backend-data", - "version": "1.0.3", + "version": "1.1.0", "license": "Apache-2.0", "dependencies": { "@aws-amplify/backend-output-schemas": "^1.1.0", @@ -27831,7 +27831,7 @@ }, "packages/backend-function": { "name": "@aws-amplify/backend-function", - "version": "1.1.0", + "version": "1.2.0", "license": "Apache-2.0", "dependencies": { "@aws-amplify/backend-output-schemas": "^1.1.0", @@ -28249,7 +28249,7 @@ }, "packages/integration-tests": { "name": "@aws-amplify/integration-tests", - "version": "0.5.5", + "version": "0.5.6", "license": "Apache-2.0", "devDependencies": { "@apollo/client": "^3.10.1", @@ -28258,6 +28258,7 @@ "@aws-amplify/backend-secret": "^1.0.0", "@aws-amplify/client-config": "^1.0.3", "@aws-amplify/data-schema": "^1.0.0", + "@aws-amplify/deployed-backend-client": "^1.0.2", "@aws-amplify/platform-core": "^1.0.1", "@aws-sdk/client-accessanalyzer": "^3.465.0", "@aws-sdk/client-amplify": "^3.465.0", @@ -28500,7 +28501,7 @@ }, "packages/schema-generator": { "name": "@aws-amplify/schema-generator", - "version": "1.1.0", + "version": "1.2.0", "license": "Apache-2.0", "dependencies": { "@aws-amplify/graphql-schema-generator": "^0.9.2", diff --git a/packages/cli/src/ampx.ts b/packages/cli/src/ampx.ts index 0d31d3c92c6..2ddb90ccb26 100755 --- a/packages/cli/src/ampx.ts +++ b/packages/cli/src/ampx.ts @@ -1,6 +1,9 @@ #!/usr/bin/env node import { createMainParser } from './main_parser_factory.js'; -import { attachUnhandledExceptionListeners } from './error_handler.js'; +import { + attachUnhandledExceptionListeners, + generateCommandFailureHandler, +} from './error_handler.js'; import { extractSubCommands } from './extract_sub_commands.js'; import { AmplifyFault, @@ -8,9 +11,9 @@ import { UsageDataEmitterFactory, } from '@aws-amplify/platform-core'; import { fileURLToPath } from 'node:url'; -import { LogLevel, format, printer } from '@aws-amplify/cli-core'; import { verifyCommandName } from './verify_command_name.js'; -import { parseAsyncSafely } from './parse_async_safely.js'; +import { hideBin } from 'yargs/helpers'; +import { format } from '@aws-amplify/cli-core'; const packageJson = new PackageJsonReader().read( fileURLToPath(new URL('../package.json', import.meta.url)) @@ -32,11 +35,11 @@ attachUnhandledExceptionListeners(usageDataEmitter); verifyCommandName(); -const parser = createMainParser(libraryVersion, usageDataEmitter); - -await parseAsyncSafely(parser); +const parser = createMainParser(libraryVersion); +const errorHandler = generateCommandFailureHandler(parser, usageDataEmitter); try { + await parser.parseAsync(hideBin(process.argv)); const metricDimension: Record = {}; const subCommands = extractSubCommands(parser); @@ -47,10 +50,6 @@ try { await usageDataEmitter.emitSuccess({}, metricDimension); } catch (e) { if (e instanceof Error) { - printer.log(format.error('Failed to emit usage metrics'), LogLevel.DEBUG); - printer.log(format.error(e), LogLevel.DEBUG); - if (e.stack) { - printer.log(e.stack, LogLevel.DEBUG); - } + await errorHandler(format.error(e), e); } } diff --git a/packages/cli/src/main_parser_factory.test.ts b/packages/cli/src/main_parser_factory.test.ts index aefec2e5632..5a54ffa4a52 100644 --- a/packages/cli/src/main_parser_factory.test.ts +++ b/packages/cli/src/main_parser_factory.test.ts @@ -1,6 +1,9 @@ import { describe, it } from 'node:test'; import assert from 'node:assert'; -import { TestCommandRunner } from './test-utils/command_runner.js'; +import { + TestCommandError, + TestCommandRunner, +} from './test-utils/command_runner.js'; import { createMainParser } from './main_parser_factory.js'; import { version } from '#package.json'; @@ -20,16 +23,29 @@ void describe('main parser', { concurrency: false }, () => { }); void it('prints help if command is not provided', async () => { - const output = await commandRunner.runCommand(''); - assert.match(output, /Commands:/); - assert.match(output, /Not enough non-option arguments:/); + await assert.rejects( + () => commandRunner.runCommand(''), + (err) => { + assert(err instanceof TestCommandError); + assert.match(err.output, /Commands:/); + assert.match(err.error.message, /Not enough non-option arguments:/); + return true; + } + ); }); void it('errors and prints help if invalid option is given', async () => { - const output = await commandRunner.runCommand( - 'sandbox --non-existing-option 1' + await assert.rejects( + () => commandRunner.runCommand('sandbox --non-existing-option 1'), + (err) => { + assert(err instanceof TestCommandError); + assert.match(err.output, /Commands:/); + assert.match( + err.error.message, + /Unknown arguments: non-existing-option/ + ); + return true; + } ); - assert.match(output, /Commands:/); - assert.match(output, /Unknown arguments: non-existing-option/); }); }); diff --git a/packages/cli/src/main_parser_factory.ts b/packages/cli/src/main_parser_factory.ts index 0747c22e8eb..5189c7de3f2 100644 --- a/packages/cli/src/main_parser_factory.ts +++ b/packages/cli/src/main_parser_factory.ts @@ -1,20 +1,15 @@ import yargs, { Argv } from 'yargs'; -import { UsageDataEmitter } from '@aws-amplify/platform-core'; import { createGenerateCommand } from './commands/generate/generate_command_factory.js'; import { createSandboxCommand } from './commands/sandbox/sandbox_command_factory.js'; import { createPipelineDeployCommand } from './commands/pipeline-deploy/pipeline_deploy_command_factory.js'; import { createConfigureCommand } from './commands/configure/configure_command_factory.js'; -import { generateCommandFailureHandler } from './error_handler.js'; import { createInfoCommand } from './commands/info/info_command_factory.js'; import * as path from 'path'; /** * Creates main parser. */ -export const createMainParser = ( - libraryVersion: string, - usageDataEmitter?: UsageDataEmitter -): Argv => { +export const createMainParser = (libraryVersion: string): Argv => { const parser = yargs() .version(libraryVersion) // This option is being used indirectly to configure the log level of the Printer instance. @@ -36,9 +31,8 @@ export const createMainParser = ( .help() .demandCommand() .strictCommands() - .recommendCommands(); - - parser.fail(generateCommandFailureHandler(parser, usageDataEmitter)); + .recommendCommands() + .fail(false); return parser; }; diff --git a/packages/cli/src/parse_async_safely.test.ts b/packages/cli/src/parse_async_safely.test.ts deleted file mode 100644 index 24f3327f158..00000000000 --- a/packages/cli/src/parse_async_safely.test.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { Argv } from 'yargs'; -import { describe, it } from 'node:test'; -import { parseAsyncSafely } from './parse_async_safely.js'; - -const mockParser: Argv = { - parseAsync: () => { - throw new Error('Mock parser error'); - }, -} as unknown as Argv; - -void describe('execute parseAsyncSafely', () => { - void it('parseAsyncSafely should not throw an error', async () => { - await parseAsyncSafely(mockParser); - //no throw - }); -}); diff --git a/packages/cli/src/parse_async_safely.ts b/packages/cli/src/parse_async_safely.ts deleted file mode 100644 index d6f229c096f..00000000000 --- a/packages/cli/src/parse_async_safely.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { Argv } from 'yargs'; -import { LogLevel, format, printer } from '@aws-amplify/cli-core'; -import { hideBin } from 'yargs/helpers'; -/** - Executes the command using the provided parser. - Handles any errors that occur during command execution. - @param parser - The parser object to parse the command line arguments. - @returns - A promise that resolves when the command execution is complete. - */ -export const parseAsyncSafely = async (parser: Argv): Promise => { - try { - await parser.parseAsync(hideBin(process.argv)); - // Yargs invoke the command failure handler before rethrowing the error.This prevents it from propagating to unhandled exception handler and being printed again. - } catch (e) { - if (e instanceof Error) { - printer.log(format.error('Failed to execute command'), LogLevel.DEBUG); - printer.log(format.error(e), LogLevel.DEBUG); - if (e.stack) { - printer.log(e.stack, LogLevel.DEBUG); - } - } - } -}; diff --git a/packages/platform-core/API.md b/packages/platform-core/API.md index d257509bf4b..79cad2c92fd 100644 --- a/packages/platform-core/API.md +++ b/packages/platform-core/API.md @@ -21,7 +21,7 @@ export abstract class AmplifyError extends Error { // (undocumented) readonly details?: string; // (undocumented) - static fromError: (error: unknown) => AmplifyError<'UnknownFault'>; + static fromError: (error: unknown) => AmplifyError<'UnknownFault' | 'CredentialsError'>; // (undocumented) static fromStderr: (_stderr: string) => AmplifyError | undefined; // (undocumented) diff --git a/packages/platform-core/src/errors/amplify_error.ts b/packages/platform-core/src/errors/amplify_error.ts index 4fbfbe34160..1634bcdbf2a 100644 --- a/packages/platform-core/src/errors/amplify_error.ts +++ b/packages/platform-core/src/errors/amplify_error.ts @@ -98,12 +98,20 @@ export abstract class AmplifyError extends Error { return undefined; }; - static fromError = (error: unknown): AmplifyError<'UnknownFault'> => { + static fromError = ( + error: unknown + ): AmplifyError<'UnknownFault' | 'CredentialsError'> => { const errorMessage = error instanceof Error ? `${error.name}: ${error.message}` : 'An unknown error happened. Check downstream error'; + if (error instanceof Error && isCredentialsError(error)) { + return new AmplifyUserError('CredentialsError', { + message: errorMessage, + resolution: '', + }); + } return new AmplifyFault( 'UnknownFault', { @@ -114,6 +122,10 @@ export abstract class AmplifyError extends Error { }; } +const isCredentialsError = (err?: Error): boolean => { + return !!err && err?.name === 'CredentialsProviderError'; +}; + /** * Amplify exception classifications */ diff --git a/packages/platform-core/src/usage-data/account_id_fetcher.test.ts b/packages/platform-core/src/usage-data/account_id_fetcher.test.ts new file mode 100644 index 00000000000..e12caa8d0ec --- /dev/null +++ b/packages/platform-core/src/usage-data/account_id_fetcher.test.ts @@ -0,0 +1,51 @@ +import { AccountIdFetcher } from './account_id_fetcher'; +import { GetCallerIdentityCommandOutput, STSClient } from '@aws-sdk/client-sts'; +import { describe, mock, test } from 'node:test'; +import assert from 'node:assert'; + +void describe('AccountIdFetcher', async () => { + void test('fetches account ID successfully', async () => { + const mockSend = mock.method(STSClient.prototype, 'send', () => + Promise.resolve({ + Account: '123456789012', + } as GetCallerIdentityCommandOutput) + ); + + const accountIdFetcher = new AccountIdFetcher(new STSClient({})); + const accountId = await accountIdFetcher.fetch(); + + assert.strictEqual(accountId, '123456789012'); + mockSend.mock.resetCalls(); + }); + + void test('returns default account ID when STS fails', async () => { + const mockSend = mock.method(STSClient.prototype, 'send', () => + Promise.reject(new Error('STS error')) + ); + + const accountIdFetcher = new AccountIdFetcher(new STSClient({})); + const accountId = await accountIdFetcher.fetch(); + + assert.strictEqual(accountId, 'NO_ACCOUNT_ID'); + mockSend.mock.resetCalls(); + }); + + void test('returns cached account ID on subsequent calls', async () => { + const mockSend = mock.method(STSClient.prototype, 'send', () => + Promise.resolve({ + Account: '123456789012', + } as GetCallerIdentityCommandOutput) + ); + + const accountIdFetcher = new AccountIdFetcher(new STSClient({})); + const accountId1 = await accountIdFetcher.fetch(); + const accountId2 = await accountIdFetcher.fetch(); + + assert.strictEqual(accountId1, '123456789012'); + assert.strictEqual(accountId2, '123456789012'); + + // we only call the service once. + assert.strictEqual(mockSend.mock.callCount(), 1); + mockSend.mock.resetCalls(); + }); +}); diff --git a/packages/platform-core/src/usage-data/account_id_fetcher.ts b/packages/platform-core/src/usage-data/account_id_fetcher.ts index 102dcdf605c..f430f9e6722 100644 --- a/packages/platform-core/src/usage-data/account_id_fetcher.ts +++ b/packages/platform-core/src/usage-data/account_id_fetcher.ts @@ -1,22 +1,32 @@ import { GetCallerIdentityCommand, STSClient } from '@aws-sdk/client-sts'; +const NO_ACCOUNT_ID = 'NO_ACCOUNT_ID'; /** * Retrieves the account ID of the user */ export class AccountIdFetcher { + private accountId: string | undefined; /** * constructor for AccountIdFetcher */ constructor(private readonly stsClient = new STSClient()) {} fetch = async () => { - const stsResponse = await this.stsClient.send( - new GetCallerIdentityCommand({}) - ); - if (stsResponse && stsResponse.Account) { - return stsResponse.Account; + if (this.accountId) { + return this.accountId; + } + try { + const stsResponse = await this.stsClient.send( + new GetCallerIdentityCommand({}) + ); + if (stsResponse && stsResponse.Account) { + this.accountId = stsResponse.Account; + return this.accountId; + } + // We failed to get the account Id. Most likely the user doesn't have credentials + return NO_ACCOUNT_ID; + } catch (error) { + // We failed to get the account Id. Most likely the user doesn't have credentials + return NO_ACCOUNT_ID; } - throw new Error( - 'Cannot retrieve the account Id from GetCallerIdentityCommand' - ); }; } diff --git a/packages/platform-core/src/usage-data/usage_data_emitter.ts b/packages/platform-core/src/usage-data/usage_data_emitter.ts index 118ce69ecf4..d108dc9fc75 100644 --- a/packages/platform-core/src/usage-data/usage_data_emitter.ts +++ b/packages/platform-core/src/usage-data/usage_data_emitter.ts @@ -29,24 +29,34 @@ export class DefaultUsageDataEmitter implements UsageDataEmitter { metrics?: Record, dimensions?: Record ) => { - const data = await this.getUsageData({ - state: 'SUCCEEDED', - metrics, - dimensions, - }); - await this.send(data); + try { + const data = await this.getUsageData({ + state: 'SUCCEEDED', + metrics, + dimensions, + }); + await this.send(data); + // eslint-disable-next-line amplify-backend-rules/no-empty-catch + } catch { + // Don't propagate errors related to not being able to send telemetry + } }; emitFailure = async ( error: AmplifyError, dimensions?: Record ) => { - const data = await this.getUsageData({ - state: 'FAILED', - error, - dimensions, - }); - await this.send(data); + try { + const data = await this.getUsageData({ + state: 'FAILED', + error, + dimensions, + }); + await this.send(data); + // eslint-disable-next-line amplify-backend-rules/no-empty-catch + } catch { + // Don't propagate errors related to not being able to send telemetry + } }; private getUsageData = async (options: { @@ -58,7 +68,7 @@ export class DefaultUsageDataEmitter implements UsageDataEmitter { return { accountId: await this.accountIdFetcher.fetch(), sessionUuid: this.sessionUuid, - installationUuid: await getInstallationUuid(), + installationUuid: getInstallationUuid(), amplifyCliVersion: this.libraryVersion, timestamp: new Date().toISOString(), error: options.error ? new SerializableError(options.error) : undefined, diff --git a/packages/sandbox/src/file_watching_sandbox.test.ts b/packages/sandbox/src/file_watching_sandbox.test.ts index f6b1781bf90..2654336a015 100644 --- a/packages/sandbox/src/file_watching_sandbox.test.ts +++ b/packages/sandbox/src/file_watching_sandbox.test.ts @@ -27,7 +27,11 @@ import { import { fileURLToPath } from 'url'; import { BackendIdentifier } from '@aws-amplify/plugin-types'; import { AmplifyUserError } from '@aws-amplify/platform-core'; -import { ParameterNotFound, SSMClient } from '@aws-sdk/client-ssm'; +import { + ParameterNotFound, + SSMClient, + SSMServiceException, +} from '@aws-sdk/client-ssm'; // Watcher mocks const unsubscribeMockFn = mock.fn(); @@ -173,6 +177,32 @@ void describe('Sandbox to check if region is bootstrapped', () => { ); }); + void it('when user does not have proper credentials throw user error', async () => { + const error = new SSMServiceException({ + name: 'UnrecognizedClientException', + $fault: 'client', + $metadata: {}, + message: 'The security token included in the request is invalid.', + }); + ssmClientSendMock.mock.mockImplementationOnce(() => { + throw error; + }); + + await assert.rejects( + () => sandboxInstance.start({}), + new AmplifyUserError( + 'SSMCredentialsError', + { + message: + 'UnrecognizedClientException: The security token included in the request is invalid.', + resolution: + 'Make sure your AWS credentials are set up correctly and have permissions to call SSM:GetParameter', + }, + error + ) + ); + }); + void it('when region has bootstrapped, but with a version lower than the minimum (6), then opens console to initiate bootstrap', async () => { ssmClientSendMock.mock.mockImplementationOnce(() => Promise.resolve({ diff --git a/packages/sandbox/src/file_watching_sandbox.ts b/packages/sandbox/src/file_watching_sandbox.ts index 46e6cb9197d..e54bc2e1236 100644 --- a/packages/sandbox/src/file_watching_sandbox.ts +++ b/packages/sandbox/src/file_watching_sandbox.ts @@ -19,6 +19,7 @@ import { GetParameterCommand, ParameterNotFound, SSMClient, + SSMServiceException, } from '@aws-sdk/client-ssm'; import { AmplifyPrompter, @@ -330,7 +331,27 @@ export class FileWatchingSandbox extends EventEmitter implements Sandbox { if (e instanceof ParameterNotFound) { return false; } - // If we are unable to retrieve bootstrap version parameter due to other reasons(AccessDenied), we fail fast. + if ( + e instanceof SSMServiceException && + [ + 'UnrecognizedClientException', + 'AccessDeniedException', + 'NotAuthorized', + 'ExpiredTokenException', + ].includes(e.name) + ) { + throw new AmplifyUserError( + 'SSMCredentialsError', + { + message: `${e.name}: ${e.message}`, + resolution: + 'Make sure your AWS credentials are set up correctly and have permissions to call SSM:GetParameter', + }, + e + ); + } + + // If we are unable to retrieve bootstrap version parameter due to other reasons, we fail fast. throw e; } };