From 0ff077344cc89331275f7e97c3244abbdd116521 Mon Sep 17 00:00:00 2001
From: jsetton <jeremy.setton@gmail.com>
Date: Thu, 9 Nov 2023 19:24:34 -0500
Subject: [PATCH] chore: improve s3 error handling

---
 .../deploy-delegates/cfn-deployer/index.js    | 13 ++++++++----
 test/unit/builtins/cfn-deployer/index-test.js | 21 +++++++++++++++----
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/lib/builtins/deploy-delegates/cfn-deployer/index.js b/lib/builtins/deploy-delegates/cfn-deployer/index.js
index 917b14e5..d916c04d 100644
--- a/lib/builtins/deploy-delegates/cfn-deployer/index.js
+++ b/lib/builtins/deploy-delegates/cfn-deployer/index.js
@@ -32,8 +32,8 @@ async function bootstrap(options, callback) {
     fs.writeFileSync(templateLocation, templateContent);
     userConfig.templatePath = `./${path.posix.join("infrastructure", path.basename(workspacePath), SKILL_STACK_PUBLIC_FILE_NAME)}`;
     updatedUserConfig = R.set(R.lensPath(["awsRegion"]), awsDefaultRegion, userConfig);
-  } catch (e) {
-    return callback(e.message);
+  } catch (err) {
+    return callback(err.message);
   }
   callback(null, {userConfig: updatedUserConfig});
 }
@@ -96,7 +96,12 @@ async function _deploy(reporter, options, deployProgress) {
     templateBody.includes("SkillClientId") && templateBody.includes("SkillClientSecret") ? await helper.getSkillCredentials(skillId) : {};
 
   // s3 upload
-  const uploadResult = await helper.uploadToS3(bucketName, bucketKey, code.codeBuild);
+  let uploadResult;
+  try {
+    uploadResult = await helper.uploadToS3(bucketName, bucketKey, code.codeBuild);
+  } catch (err) {
+    throw new CliCFNDeployerError(`Failed to upload code build to S3: ${err.message}`);
+  }
 
   deployProgress.deployState.s3 = {
     bucket: bucketName,
@@ -212,7 +217,7 @@ function _getUserDefinedParameters(alexaRegion, userConfig) {
   Object.keys(parameters).forEach((key) => {
     if (reservedParametersKeys.has(key)) {
       const message = reservedParameters[key];
-      throw new CliCFNDeployerError(`Cloud Formation parameter "${key}" is reserved. ${message}`);
+      throw new CliCFNDeployerError(`CloudFormation parameter "${key}" is reserved. ${message}`);
     }
   });
 
diff --git a/test/unit/builtins/cfn-deployer/index-test.js b/test/unit/builtins/cfn-deployer/index-test.js
index eec36098..c44b8357 100644
--- a/test/unit/builtins/cfn-deployer/index-test.js
+++ b/test/unit/builtins/cfn-deployer/index-test.js
@@ -22,7 +22,7 @@ describe("Builtins test - cfn-deployer index test", () => {
   const endpointUri = "some endpoint uri";
   const userDefinedParamKey = "someKey";
   const userDefinedParamValue = "someValue";
-  let deployStackStub, getAWSProfileStub;
+  let deployStackStub, getAWSProfileStub, uploadToS3Stub;
 
   describe("bootstrap", () => {
     const bootstrapOptions = {
@@ -118,7 +118,7 @@ describe("Builtins test - cfn-deployer index test", () => {
       };
       getAWSProfileStub = sinon.stub(awsUtil, "getAWSProfile").returns("some profile");
       sinon.stub(Helper.prototype, "getSkillCredentials").resolves({clientId: "id", clientSecret: "secret"});
-      sinon.stub(Helper.prototype, "uploadToS3").resolves({VersionId: s3VersionId});
+      uploadToS3Stub = sinon.stub(Helper.prototype, "uploadToS3").resolves({VersionId: s3VersionId});
       deployStackStub = sinon.stub(Helper.prototype, "deployStack");
     });
 
@@ -227,13 +227,26 @@ describe("Builtins test - cfn-deployer index test", () => {
       });
     });
 
+    it("should throw error when failed to upload to s3", (done) => {
+      const errorMessage = "some error";
+      uploadToS3Stub.rejects(new Error(errorMessage));
+
+      Deployer.invoke({}, deployOptions, (err, result) => {
+        expectedErrorOutput.resultMessage =
+          `The CloudFormation deploy failed for Alexa region "${alexaRegion}": Failed to upload code build to S3: ${errorMessage}`;
+        expect(err).eql(null);
+        expect(result).eql(expectedErrorOutput);
+        done();
+      });
+    });
+
     it("should throw error when reserved parameter is used", (done) => {
       deployOptions.userConfig.cfn.parameters.SkillId = "reserved parameter value";
 
       Deployer.invoke({}, deployOptions, (err, result) => {
         expectedErrorOutput.resultMessage =
           `The CloudFormation deploy failed for Alexa region "${alexaRegion}": ` +
-          'Cloud Formation parameter "SkillId" is reserved. Please use a different name.';
+          'CloudFormation parameter "SkillId" is reserved. Please use a different name.';
         expect(err).eql(null);
         expect(result).eql(expectedErrorOutput);
         done();
@@ -245,7 +258,7 @@ describe("Builtins test - cfn-deployer index test", () => {
 
       Deployer.invoke({}, deployOptions, (err, result) => {
         expectedErrorOutput.resultMessage =
-          `The CloudFormation deploy failed for Alexa region "${alexaRegion}": ` + "The template path in userConfig must be provided.";
+          `The CloudFormation deploy failed for Alexa region "${alexaRegion}": The template path in userConfig must be provided.`;
         expect(err).eql(null);
         expect(result).eql(expectedErrorOutput);
         done();