Skip to content

Commit

Permalink
feat(cloudfront): responseHttpStatus defaults to httpStatus in errorR…
Browse files Browse the repository at this point in the history
…esponses (#11879)

Make `responseHttpStatus` default to `httpStatus` if `responsePagePath`
is defined.

This avoids repeating the error code and is more in line with the actual
`@default` documentation (which actually led me to think that it
already defaulted to `httpStatus`).


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
jogold authored Dec 7, 2020
1 parent 60875a5 commit c6052ae
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 27 deletions.
19 changes: 8 additions & 11 deletions packages/@aws-cdk/aws-cloudfront/lib/distribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -393,21 +393,18 @@ export class Distribution extends Resource implements IDistribution {

private renderErrorResponses(): CfnDistribution.CustomErrorResponseProperty[] | undefined {
if (this.errorResponses.length === 0) { return undefined; }
function validateCustomErrorResponse(errorResponse: ErrorResponse) {
if (errorResponse.responsePagePath && !errorResponse.responseHttpStatus) {
throw new Error('\'responseCode\' must be provided if \'responsePagePath\' is defined');
}
if (!errorResponse.responseHttpStatus && !errorResponse.ttl) {
throw new Error('A custom error response without either a \'responseCode\' or \'errorCachingMinTtl\' is not valid.');
}
}
this.errorResponses.forEach(e => validateCustomErrorResponse(e));

return this.errorResponses.map(errorConfig => {
if (!errorConfig.responseHttpStatus && !errorConfig.ttl && !errorConfig.responsePagePath) {
throw new Error('A custom error response without either a \'responseHttpStatus\', \'ttl\' or \'responsePagePath\' is not valid.');
}

return {
errorCachingMinTtl: errorConfig.ttl?.toSeconds(),
errorCode: errorConfig.httpStatus,
responseCode: errorConfig.responseHttpStatus,
responseCode: errorConfig.responsePagePath
? errorConfig.responseHttpStatus ?? errorConfig.httpStatus
: errorConfig.responseHttpStatus,
responsePagePath: errorConfig.responsePagePath,
};
});
Expand Down Expand Up @@ -577,7 +574,7 @@ export interface ErrorResponse {
*
* If you specify a value for `responseHttpStatus`, you must also specify a value for `responsePagePath`.
*
* @default - not set, the error code will be returned as the response code.
* @default - the error code will be returned as the response code.
*/
readonly responseHttpStatus?: number;
/**
Expand Down
30 changes: 14 additions & 16 deletions packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,20 +318,6 @@ describe('certificates', () => {

describe('custom error responses', () => {

test('should fail if responsePagePath is defined but responseCode is not', () => {
const origin = defaultOrigin();

expect(() => {
new Distribution(stack, 'Dist', {
defaultBehavior: { origin },
errorResponses: [{
httpStatus: 404,
responsePagePath: '/errors/404.html',
}],
});
}).toThrow(/\'responseCode\' must be provided if \'responsePagePath\' is defined/);
});

test('should fail if only the error code is provided', () => {
const origin = defaultOrigin();

Expand All @@ -340,21 +326,28 @@ describe('custom error responses', () => {
defaultBehavior: { origin },
errorResponses: [{ httpStatus: 404 }],
});
}).toThrow(/A custom error response without either a \'responseCode\' or \'errorCachingMinTtl\' is not valid./);
}).toThrow(/A custom error response without either a \'responseHttpStatus\', \'ttl\' or \'responsePagePath\' is not valid./);
});

test('should render the array of error configs if provided', () => {
const origin = defaultOrigin();
new Distribution(stack, 'Dist', {
defaultBehavior: { origin },
errorResponses: [{
// responseHttpStatus defaults to httpsStatus
httpStatus: 404,
responseHttpStatus: 404,
responsePagePath: '/errors/404.html',
},
{
// without responsePagePath
httpStatus: 500,
ttl: Duration.seconds(2),
},
{
// with responseHttpStatus different from httpStatus
httpStatus: 403,
responseHttpStatus: 200,
responsePagePath: '/index.html',
}],
});

Expand All @@ -370,6 +363,11 @@ describe('custom error responses', () => {
ErrorCachingMinTTL: 2,
ErrorCode: 500,
},
{
ErrorCode: 403,
ResponseCode: 200,
ResponsePagePath: '/index.html',
},
],
},
});
Expand Down

0 comments on commit c6052ae

Please sign in to comment.