From 21d81a8524ac3a4107606e11342c057272872baf Mon Sep 17 00:00:00 2001 From: Penghao He Date: Wed, 4 Nov 2020 15:05:57 -0800 Subject: [PATCH] chore: add retry to EnvController waitFor requests (#1632) Address this [comment](https://github.com/aws/copilot-cli/pull/1508#discussion_r505919833) left in #1508. Add retry to EnvController when we call `cfn.waitFor()`, so that it won't immediately error out when the request gets throttled. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. --- cf-custom-resources/lib/env-controller.js | 62 +++++++++++++------ .../test/env-controller-test.js | 5 +- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/cf-custom-resources/lib/env-controller.js b/cf-custom-resources/lib/env-controller.js index ee3362d14a9..a66ffda6af9 100644 --- a/cf-custom-resources/lib/env-controller.js +++ b/cf-custom-resources/lib/env-controller.js @@ -7,6 +7,11 @@ const aws = require("aws-sdk"); // These are used for test purposes only let defaultResponseURL; +const updateStackWaiter = { + delay: 30, + maxAttempts: 29, +}; + /** * Upload a CloudFormation response object to S3. * @@ -136,6 +141,7 @@ const controlEnv = async function ( await cfn .waitFor("stackUpdateComplete", { StackName: stackName, + $waiter: updateStackWaiter, }) .promise(); continue; @@ -144,6 +150,7 @@ const controlEnv = async function ( await cfn .waitFor("stackUpdateComplete", { StackName: stackName, + $waiter: updateStackWaiter, }) .promise(); describeStackResp = await cfn @@ -169,30 +176,39 @@ exports.handler = async function (event, context) { try { switch (event.RequestType) { case "Create": - responseData = await controlEnv( - "Create", - props.EnvStack, - props.Workload, - props.Parameters - ); + responseData = await Promise.race([ + exports.deadlineExpired(), + controlEnv( + "Create", + props.EnvStack, + props.Workload, + props.Parameters + ), + ]); physicalResourceId = `envcontoller/${props.EnvStack}/${props.Workload}`; break; case "Update": - responseData = await controlEnv( - "Update", - props.EnvStack, - props.Workload, - props.Parameters - ); + responseData = await Promise.race([ + exports.deadlineExpired(), + controlEnv( + "Update", + props.EnvStack, + props.Workload, + props.Parameters + ), + ]); physicalResourceId = event.PhysicalResourceId; break; case "Delete": - await controlEnv( - "Delete", - props.EnvStack, - props.Workload, - props.Parameters - ); + responseData = await Promise.race([ + exports.deadlineExpired(), + controlEnv( + "Delete", + props.EnvStack, + props.Workload, + props.Parameters + ), + ]); physicalResourceId = event.PhysicalResourceId; break; default: @@ -253,6 +269,16 @@ const updateParameter = function (requestType, workload, paramValue) { return [updatedParamValue, updatedParamValue !== paramValue]; }; +exports.deadlineExpired = function () { + return new Promise(function (resolve, reject) { + setTimeout( + reject, + 14 * 60 * 1000 + 30 * 1000 /* 14.5 minutes*/, + new Error("Lambda took longer than 14.5 minutes to update environment") + ); + }); +}; + /** * @private */ diff --git a/cf-custom-resources/test/env-controller-test.js b/cf-custom-resources/test/env-controller-test.js index ba6fc167296..d5af301953f 100644 --- a/cf-custom-resources/test/env-controller-test.js +++ b/cf-custom-resources/test/env-controller-test.js @@ -30,6 +30,9 @@ describe("Env Controller Handler", () => { beforeEach(() => { EnvController.withDefaultResponseURL(ResponseURL); + EnvController.deadlineExpired = function () { + return new Promise(function (resolve, reject) {}); + }; // Prevent logging. console.log = function () {}; }); @@ -208,7 +211,7 @@ describe("Env Controller Handler", () => { { StackName: "mockEnvStack", Parameters: testParams, - Outputs: [] + Outputs: [], }, ], });