Skip to content

Commit

Permalink
Refactor error handling, introduce two new AmplifyErrors (#744)
Browse files Browse the repository at this point in the history
* Refactor error handling, introduce two new AmplifyErrors

* change downstreamerror to cause

* lint + api change

* does this work?

* update comment

* Does this work then?

* This should work

* add cause to super constructor

* wrap invokeCDK in try catch

* fix tests
  • Loading branch information
Amplifiyer authored Nov 29, 2023
1 parent cd672ba commit db775ad
Show file tree
Hide file tree
Showing 46 changed files with 686 additions and 134 deletions.
11 changes: 11 additions & 0 deletions .changeset/curly-toes-doubt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'@aws-amplify/integration-tests': patch
'@aws-amplify/backend-deployer': patch
'create-amplify': patch
'@aws-amplify/platform-core': patch
'@aws-amplify/backend-data': patch
'@aws-amplify/sandbox': patch
'@aws-amplify/backend-cli': patch
---

Refactor error handling, introduce two new AmplifyErrors
65 changes: 55 additions & 10 deletions packages/backend-data/src/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
isUsingDefaultApiKeyAuth,
} from './convert_authorization_modes.js';
import { validateAuthorizationModes } from './validate_authorization_modes.js';
import { AmplifyUserError } from '@aws-amplify/platform-core';

/**
* Singleton factory for AmplifyGraphqlApi constructs that can be used in Amplify project files
Expand Down Expand Up @@ -80,16 +81,44 @@ class DataGenerator implements ConstructContainerEntryGenerator {
) {}

generateContainerEntry = (scope: Construct) => {
const authorizationModes = convertAuthorizationModesToCDK(
this.functionInstanceProvider,
this.providedAuthConfig,
this.props.authorizationModes
);
let authorizationModes;

validateAuthorizationModes(
this.props.authorizationModes,
authorizationModes
);
try {
authorizationModes = convertAuthorizationModesToCDK(
this.functionInstanceProvider,
this.providedAuthConfig,
this.props.authorizationModes
);
} catch (error) {
throw new AmplifyUserError(
'InvalidSchemaAuthError',
{
message:
error instanceof Error
? error.message
: 'Cannot covert authorization modes',
},
error instanceof Error ? error : undefined
);
}

try {
validateAuthorizationModes(
this.props.authorizationModes,
authorizationModes
);
} catch (error) {
throw new AmplifyUserError(
'InvalidSchemaAuthError',
{
message:
error instanceof Error
? error.message
: 'Failed to validate authorization modes',
},
error instanceof Error ? error : undefined
);
}

const sandboxModeEnabled = isUsingDefaultApiKeyAuth(
this.providedAuthConfig,
Expand All @@ -101,9 +130,25 @@ class DataGenerator implements ConstructContainerEntryGenerator {
this.props.functions ?? {}
);

let amplifyGraphqlDefinition;
try {
amplifyGraphqlDefinition = convertSchemaToCDK(this.props.schema);
} catch (error) {
throw new AmplifyUserError(
'InvalidSchemaError',
{
message:
error instanceof Error
? error.message
: 'Cannot covert user schema',
},
error instanceof Error ? error : undefined
);
}

return new AmplifyData(scope, this.defaultName, {
apiName: this.props.name,
definition: convertSchemaToCDK(this.props.schema),
definition: amplifyGraphqlDefinition,
authorizationModes,
outputStorageStrategy: this.outputStorageStrategy,
functionNameMap,
Expand Down
9 changes: 5 additions & 4 deletions packages/backend-deployer/src/cdk_deployer.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { after, beforeEach, describe, it, mock } from 'node:test';
import { CDKDeployer } from './cdk_deployer.js';
import assert from 'node:assert';
import { BackendLocator } from '@aws-amplify/platform-core';
import { AmplifyError, BackendLocator } from '@aws-amplify/platform-core';
import { DeployProps } from './cdk_deployer_singleton_factory.js';
import { CdkErrorMapper } from './cdk_error_mapper.js';
import { BackendIdentifier } from '@aws-amplify/plugin-types';
Expand Down Expand Up @@ -269,12 +269,13 @@ void describe('invokeCDKCommand', () => {

await assert.rejects(
() => invoker.deploy(branchBackendId, sandboxDeployProps),
(err: Error) => {
(err: AmplifyError) => {
assert.equal(
err.message,
'[AccessDenied]: The deployment role does not have sufficient permissions to perform this deployment.'
'The deployment role does not have sufficient permissions to perform this deployment.'
);
assert.equal((err.cause as Error).message, 'Access Denied');
assert.equal(err.name, 'AccessDeniedError');
assert.equal(err.cause?.message, 'Access Denied');
return true;
}
);
Expand Down
71 changes: 54 additions & 17 deletions packages/backend-deployer/src/cdk_deployer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import {
} from './cdk_deployer_singleton_factory.js';
import { CdkErrorMapper } from './cdk_error_mapper.js';
import { BackendIdentifier } from '@aws-amplify/plugin-types';
import { BackendLocator, CDKContextKey } from '@aws-amplify/platform-core';
import {
AmplifyUserError,
BackendLocator,
CDKContextKey,
} from '@aws-amplify/platform-core';
import { dirname } from 'path';

/**
Expand Down Expand Up @@ -49,14 +53,18 @@ export class CDKDeployer implements BackendDeployer {
}
}

return this.invokeCdk(InvokableCommand.DEPLOY, backendId, cdkCommandArgs);
return this.tryInvokeCdk(
InvokableCommand.DEPLOY,
backendId,
cdkCommandArgs
);
};

/**
* Invokes cdk destroy command
*/
destroy = async (backendId: BackendIdentifier) => {
return this.invokeCdk(InvokableCommand.DESTROY, backendId, ['--force']);
return this.tryInvokeCdk(InvokableCommand.DESTROY, backendId, ['--force']);
};

private invokeTsc = async (deployProps?: DeployProps) => {
Expand All @@ -78,14 +86,44 @@ export class CDKDeployer implements BackendDeployer {
// If we cannot load ts config, turn off type checking
return;
}
await this.executeChildProcess('npx', [
'tsc',
'--noEmit',
'--skipLibCheck',
// pointing the project arg to the amplify backend directory will use the tsconfig present in that directory
'--project',
dirname(this.backendLocator.locate()),
]);
try {
await this.executeChildProcess('npx', [
'tsc',
'--noEmit',
'--skipLibCheck',
// pointing the project arg to the amplify backend directory will use the tsconfig present in that directory
'--project',
dirname(this.backendLocator.locate()),
]);
} catch (err) {
throw new AmplifyUserError(
'SyntaxError',
{
message:
'TypeScript validation check failed, check your backend definition',
},
err instanceof Error ? err : undefined
);
}
};

/**
* calls invokeCDK and wrap it in a try catch
*/
private tryInvokeCdk = async (
invokableCommand: InvokableCommand,
backendId: BackendIdentifier,
additionalArguments?: string[]
): Promise<DeployResult | DestroyResult> => {
try {
return await this.invokeCdk(
invokableCommand,
backendId,
additionalArguments
);
} catch (err) {
throw this.cdkErrorMapper.getAmplifyError(err as Error);
}
};

/**
Expand Down Expand Up @@ -131,11 +169,7 @@ export class CDKDeployer implements BackendDeployer {
cdkCommandArgs.push(...additionalArguments);
}

try {
return await this.executeChildProcess('npx', cdkCommandArgs);
} catch (err) {
throw this.cdkErrorMapper.getHumanReadableError(err as Error);
}
return await this.executeChildProcess('npx', cdkCommandArgs);
};

/**
Expand Down Expand Up @@ -183,7 +217,10 @@ export class CDKDeployer implements BackendDeployer {
await childProcess;
return cdkOutput;
} catch (error) {
// swallow execa error which is not really helpful, rather throw stderr
// swallow execa error which is most of the time noise (basically child exited with exit code...)
// bubbling this up to customers add confusion (Customers don't need to know we are running IPC calls
// and their exit codes printed while sandbox continue to run). Hence we explicitly don't pass error in the cause
// rather throw the entire stderr for clients to figure out what to do with it.
throw new Error(aggregatedStderr);
}
};
Expand Down
59 changes: 47 additions & 12 deletions packages/backend-deployer/src/cdk_error_mapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,52 +5,85 @@ import { CdkErrorMapper } from './cdk_error_mapper.js';
const testErrorMappings = [
{
errorMessage: 'UnknownError',
expectedTopLevelErrorMessage: 'UnknownError',
expectedTopLevelErrorMessage: 'Error: UnknownError',
errorName: 'UnknownFault',
expectedDownstreamErrorMessage: undefined,
},
{
errorMessage: 'ExpiredToken',
expectedTopLevelErrorMessage:
'[ExpiredToken]: The security token included in the request is invalid.',
'The security token included in the request is invalid.',
errorName: 'ExpiredTokenError',
expectedDownstreamErrorMessage: 'ExpiredToken',
},
{
errorMessage: 'Access Denied',
expectedTopLevelErrorMessage:
'[AccessDenied]: The deployment role does not have sufficient permissions to perform this deployment.',
'The deployment role does not have sufficient permissions to perform this deployment.',
errorName: 'AccessDeniedError',
expectedDownstreamErrorMessage: 'Access Denied',
},
{
errorMessage: 'ReferenceError: var is not defined\n',
expectedTopLevelErrorMessage:
'Unable to build Amplify backend. Check your backend definition in the `amplify` folder.',
errorName: 'SyntaxError',
expectedDownstreamErrorMessage: 'ReferenceError: var is not defined\n',
},
{
errorMessage: 'Has the environment been bootstrapped',
expectedTopLevelErrorMessage:
'[BootstrapFailure]: This AWS account and region has not been bootstrapped. Run `cdk bootstrap aws://{YOUR_ACCOUNT_ID}/{YOUR_REGION}` locally to resolve this.',
'This AWS account and region has not been bootstrapped. Run `cdk bootstrap aws://{YOUR_ACCOUNT_ID}/{YOUR_REGION}` locally to resolve this.',
errorName: 'BootstrapNotDetectedError',
expectedDownstreamErrorMessage: 'Has the environment been bootstrapped',
},
{
errorMessage: 'Amplify Backend not found in amplify/backend.ts',
expectedTopLevelErrorMessage:
'Backend definition could not be found in amplify directory',
errorName: 'FileConventionError',
expectedDownstreamErrorMessage:
'Amplify Backend not found in amplify/backend.ts',
},
{
errorMessage: 'Amplify Auth must be defined in amplify/auth/resource.ts',
expectedTopLevelErrorMessage:
'File name or path for backend definition are incorrect',
errorName: 'FileConventionError',
expectedDownstreamErrorMessage:
'Amplify Auth must be defined in amplify/auth/resource.ts',
},
{
errorMessage: 'amplify/backend.ts',
expectedTopLevelErrorMessage:
'[SynthError]: Unable to build Amplify backend. Check your backend definition in the `amplify` folder.',
'Unable to build Amplify backend. Check your backend definition in the `amplify` folder.',
errorName: 'BackendBuildError',
expectedDownstreamErrorMessage: 'amplify/backend.ts',
},
{
errorMessage: '❌ Deployment failed: something bad happened\n',
errorMessage:
'Overall error message had other stuff before ❌ Deployment failed: something bad happened\n and after',
expectedTopLevelErrorMessage:
'[CloudFormationFailure]: The CloudFormation deployment has failed. Find more information in the CloudFormation AWS Console for this stack.',
expectedDownstreamErrorMessage: 'something bad happened',
'The CloudFormation deployment has failed. Find more information in the CloudFormation AWS Console for this stack.',
errorName: 'CloudFormationDeploymentError',
expectedDownstreamErrorMessage:
'❌ Deployment failed: something bad happened\n',
},
{
errorMessage:
'CFN error happened: Updates are not allowed for property: some property',
expectedTopLevelErrorMessage:
'[UpdateNotSupported]: The changes that you are trying to apply are not supported.',
'The changes that you are trying to apply are not supported.',
errorName: 'CFNUpdateNotSupportedError',
expectedDownstreamErrorMessage:
'CFN error happened: Updates are not allowed for property: some property',
},
{
errorMessage:
'CFN error happened: Invalid AttributeDataType input, consider using the provided AttributeDataType enum',
expectedTopLevelErrorMessage:
'[UpdateNotSupported]: User pool attributes cannot be changed after a user pool has been created.',
'User pool attributes cannot be changed after a user pool has been created.',
errorName: 'CFNUpdateNotSupportedError',
expectedDownstreamErrorMessage:
'CFN error happened: Invalid AttributeDataType input, consider using the provided AttributeDataType enum',
},
Expand All @@ -62,16 +95,18 @@ void describe('invokeCDKCommand', { concurrency: 1 }, () => {
({
errorMessage,
expectedTopLevelErrorMessage,
errorName: expectedErrorName,
expectedDownstreamErrorMessage,
}) => {
void it(`handles ${errorMessage} error`, () => {
const humanReadableError = cdkErrorMapper.getHumanReadableError(
const humanReadableError = cdkErrorMapper.getAmplifyError(
new Error(errorMessage)
);
assert.equal(humanReadableError.message, expectedTopLevelErrorMessage);
assert.equal(humanReadableError.name, expectedErrorName);
expectedDownstreamErrorMessage &&
assert.equal(
(humanReadableError.cause as Error).message,
humanReadableError.cause?.message,
expectedDownstreamErrorMessage
);
});
Expand Down
Loading

0 comments on commit db775ad

Please sign in to comment.