Do not abort SAR loop on throttling

Author: marekaiv

samtranslator/plugins/application/serverless_app_plugin.py

@@ -180,6 +180,9 @@ def _handle_get_application_request(self, app_id, semver, key, logical_id):
             warning_message = "{}. Unable to verify access to {}/{}.".format(e, app_id, semver)
             LOG.warning(warning_message)
             self._applications[key] = {"Unable to verify"}
+        except ClientError as e:
+            LOG.exception(e)
+            raise e
 
     def _handle_create_cfn_template_request(self, app_id, semver, key, logical_id):
         """
@@ -194,7 +197,12 @@ def _handle_create_cfn_template_request(self, app_id, semver, key, logical_id):
         create_cfn_template = lambda app_id, semver: self._sar_client.create_cloud_formation_template(
             ApplicationId=self._sanitize_sar_str_param(app_id), SemanticVersion=self._sanitize_sar_str_param(semver)
         )
-        response = self._sar_service_call(create_cfn_template, logical_id, app_id, semver)
+        try:
+            response = self._sar_service_call(create_cfn_template, logical_id, app_id, semver)
+        except ClientError as e:
+            LOG.exception(e)
+            raise e
+
         LOG.info("Requested to create CFN template {}/{} in serverless application repo.".format(app_id, semver))
         self._applications[key] = response[self.TEMPLATE_URL_KEY]
         if response["Status"] != "ACTIVE":
@@ -308,6 +316,7 @@ def on_after_transform_template(self, template):
             while (time() - start_time) < self.TEMPLATE_WAIT_TIMEOUT_SECONDS:
                 temp = self._in_progress_templates
                 self._in_progress_templates = []
+                throttled = False
 
                 # Check each resource to make sure it's active
                 LOG.info("Checking resources in serverless application repo...")
@@ -318,8 +327,26 @@ def on_after_transform_template(self, template):
                             TemplateId=self._sanitize_sar_str_param(template_id),
                         )
                     )
-                    response = self._sar_service_call(get_cfn_template, application_id, application_id, template_id)
+                    response = {"Status": "PREPARING"}  # default response if we can't reach SAR
+
+                    try:
+                        if not throttled:
+                            response = self._sar_service_call(
+                                get_cfn_template, application_id, application_id, template_id
+                            )
+                    except ClientError as e:
+                        error_code = e.response["Error"]["Code"]
+                        if error_code == "TooManyRequestsException":
+                            # We were throttled by SAR, don't hammer SAR with more calls in this for loop
+                            throttled = True
+                            LOG.debug("SAR call timed out for application id {}".format(application_id))
+                            # don't re-raise, fall through to regular processing as if the template wasn't ready yet
+                        else:
+                            LOG.exception(e)
+                            raise e
+
                     self._handle_get_cfn_template_response(response, application_id, template_id)
+
                 LOG.info("Finished checking resources in serverless application repo.")
 
                 # Don't sleep if there are no more templates with PREPARING status
@@ -372,9 +399,6 @@ def _sar_service_call(self, service_call_lambda, logical_id, *args):
             error_code = e.response["Error"]["Code"]
             if error_code in ("AccessDeniedException", "NotFoundException"):
                 raise InvalidResourceException(logical_id, e.response["Error"]["Message"])
-
-            # 'ForbiddenException'- SAR rejects connection
-            LOG.exception(e)
             raise e
 
     def _resource_is_supported(self, resource_type):

tests/plugins/application/test_serverless_app_plugin.py

@@ -1,12 +1,14 @@
 import boto3
 import itertools
+from botocore.exceptions import ClientError
 
 from mock import Mock, patch
 from unittest import TestCase
 from parameterized import parameterized, param
 
 from samtranslator.plugins.application.serverless_app_plugin import ServerlessAppPlugin
 from samtranslator.plugins.exceptions import InvalidPluginException
+from samtranslator.model.exceptions import InvalidResourceException
 
 # TODO: run tests when AWS CLI is not configured (so they can run in brazil)
 
@@ -253,9 +255,53 @@ def __init__(self, app_id="app_id", semver="1.3.5"):
 
 #     self.plugin.on_before_transform_resource(app_resources[0][0], 'AWS::Serverless::Application', app_resources[0][1].properties)
 
-# class TestServerlessAppPlugin_on_after_transform_template(TestCase):
 
-#     def setUp(self):
-#         self.plugin = SeverlessAppPlugin()
-
-# # TODO: test this lifecycle event
+class TestServerlessAppPlugin_on_after_transform_template(TestCase):
+    def setUp(self):
+        pass
+
+    def test_sar_throttling_doesnt_stop_processing(self):
+        client = Mock()
+        client.get_cloud_formation_template = Mock()
+        client.get_cloud_formation_template.side_effect = ClientError(
+            {"Error": {"Code": "TooManyRequestsException"}}, "GetCloudFormationTemplate"
+        )
+        plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
+        plugin._in_progress_templates = [("appid", "template")]
+        plugin.SLEEP_TIME_SECONDS = 0.05
+        plugin.TEMPLATE_WAIT_TIMEOUT_SECONDS = 0.3
+        with self.assertRaises(InvalidResourceException):
+            plugin.on_after_transform_template("template")
+        # confirm we had at least two attempts to call SAR
+        self.assertGreater(client.get_cloud_formation_template.call_count, 1)
+
+    def test_unexpected_sar_error_stops_processing(self):
+        client = Mock()
+        client.get_cloud_formation_template = Mock()
+        client.get_cloud_formation_template.side_effect = ClientError(
+            {"Error": {"Code": "BadBadError"}}, "GetCloudFormationTemplate"
+        )
+        plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
+        plugin._in_progress_templates = [("appid", "template")]
+        with self.assertRaises(ClientError):
+            plugin.on_after_transform_template("template")
+
+    def test_sar_success_one_app(self):
+        client = Mock()
+        client.get_cloud_formation_template = Mock()
+        client.get_cloud_formation_template.return_value = {"Status": "ACTIVE"}
+        plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
+        plugin._in_progress_templates = [("appid", "template")]
+        plugin.on_after_transform_template("template")
+        # should have exactly one call to SAR
+        self.assertEqual(client.get_cloud_formation_template.call_count, 1)
+
+    def test_sar_success_two_apps(self):
+        client = Mock()
+        client.get_cloud_formation_template = Mock()
+        client.get_cloud_formation_template.return_value = {"Status": "ACTIVE"}
+        plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
+        plugin._in_progress_templates = [("appid1", "template1"), ("appid2", "template2")]
+        plugin.on_after_transform_template("template")
+        # should have exactly one call to SAR per app
+        self.assertEqual(client.get_cloud_formation_template.call_count, 2)