Merge pull request #3086 from alphagov/lambda-errors

Failover to second lambda on error

app/clients/cbc_proxy.py

@@ -51,7 +51,15 @@ class CBCProxyClient:
 
 
 class CBCProxyClientBase(ABC):
-    lambda_name = None
+    @property
+    @abstractmethod
+    def lambda_name(self):
+        pass
+
+    @property
+    @abstractmethod
+    def failover_lambda_name(self):
+        pass
 
     @property
     @abstractmethod
@@ -100,28 +108,42 @@ class CBCProxyClientBase(ABC):
     ):
         pass
 
-    def _invoke_lambda(self, payload):
-        if not self.lambda_name:
-            current_app.logger.warning(
-                '{self.__class__.__name__} tried to send {payload} but cbc proxy aws env vars not set'
-            )
-            return
+    def _invoke_lambda_with_failover(self, payload):
+        result = self._invoke_lambda(self.lambda_name, payload)
 
+        if not result:
+            failover_result = self._invoke_lambda(self.failover_lambda_name, payload)
+            if not failover_result:
+                raise CBCProxyException(f'Lambda failed for both {self.lambda_name} and {self.failover_lambda_name}')
+
+        return result
+
+    def _invoke_lambda(self, lambda_name, payload):
         payload_bytes = bytes(json.dumps(payload), encoding='utf8')
-
+        current_app.logger.info(f"Calling lambda {lambda_name}")
         result = self._lambda_client.invoke(
-            FunctionName=self.lambda_name,
+            FunctionName=lambda_name,
             InvocationType='RequestResponse',
             Payload=payload_bytes,
         )
+        current_app.logger.info(f"Finished calling lambda {lambda_name}")
 
         if result['StatusCode'] > 299:
-            raise CBCProxyException('Could not invoke lambda')
+            current_app.logger.info(
+                f"Error calling lambda {self.lambda_name} with status code { result['StatusCode']}, {result.get('Payload')}"
+            )
+            success = False
 
-        if 'FunctionError' in result:
-            raise CBCProxyException('Function exited with unhandled exception')
+        elif 'FunctionError' in result:
+            current_app.logger.info(
+                f"Error calling lambda {self.lambda_name} with function error { result['Payload'] }"
+            )
+            success = False
 
-        return result
+        else:
+            success = True
+
+        return success
 
     def infer_language_from(self, content):
         if non_gsm_characters(content):
@@ -135,6 +157,9 @@ class CBCProxyCanary(CBCProxyClientBase):
     canary, a specific lambda that does not open a vpn or connect to a provider but just responds from within AWS.
     """
     lambda_name = 'canary'
+    # we don't need a failover lambda for the canary as it doesn't actually make calls out to a CBC
+    # so we just reuse the normal one in case of a failover scenario
+    failover_lambda_name = 'canary'
 
     LANGUAGE_ENGLISH = None
     LANGUAGE_WELSH = None
@@ -143,11 +168,12 @@ class CBCProxyCanary(CBCProxyClientBase):
         self,
         identifier,
     ):
-        self._invoke_lambda(payload={'identifier': identifier})
+        self._invoke_lambda(self.lambda_name, payload={'identifier': identifier})
 
 
 class CBCProxyEE(CBCProxyClientBase):
     lambda_name = 'bt-ee-1-proxy'
+    failover_lambda_name = 'bt-ee-2-proxy'
 
     LANGUAGE_ENGLISH = 'en-GB'
     LANGUAGE_WELSH = 'cy-GB'
@@ -167,7 +193,7 @@ class CBCProxyEE(CBCProxyClientBase):
             'message_format': 'cap'
         }
 
-        self._invoke_lambda(payload=payload)
+        self._invoke_lambda_with_failover(payload=payload)
 
     def create_and_send_broadcast(
         self, identifier, headline, description, areas, sent, expires, message_number=None
@@ -183,7 +209,7 @@ class CBCProxyEE(CBCProxyClientBase):
             'expires': expires,
             'language': self.infer_language_from(description),
         }
-        self._invoke_lambda(payload=payload)
+        self._invoke_lambda_with_failover(payload=payload)
 
     def cancel_broadcast(
         self,
@@ -202,11 +228,12 @@ class CBCProxyEE(CBCProxyClientBase):
             ],
             'sent': sent,
         }
-        self._invoke_lambda(payload=payload)
+        self._invoke_lambda_with_failover(payload=payload)
 
 
 class CBCProxyVodafone(CBCProxyClientBase):
     lambda_name = 'vodafone-1-proxy'
+    failover_lambda_name = 'vodafone-2-proxy'
 
     LANGUAGE_ENGLISH = 'English'
     LANGUAGE_WELSH = 'Welsh'
@@ -227,7 +254,7 @@ class CBCProxyVodafone(CBCProxyClientBase):
             'message_format': 'ibag'
         }
 
-        self._invoke_lambda(payload=payload)
+        self._invoke_lambda_with_failover(payload=payload)
 
     def create_and_send_broadcast(
         self, identifier, message_number, headline, description, areas, sent, expires,
@@ -244,7 +271,7 @@ class CBCProxyVodafone(CBCProxyClientBase):
             'expires': expires,
             'language': self.infer_language_from(description),
         }
-        self._invoke_lambda(payload=payload)
+        self._invoke_lambda_with_failover(payload=payload)
 
     def cancel_broadcast(
         self, identifier, previous_provider_messages, sent, message_number
@@ -264,4 +291,4 @@ class CBCProxyVodafone(CBCProxyClientBase):
             ],
             'sent': sent,
         }
-        self._invoke_lambda(payload=payload)
+        self._invoke_lambda_with_failover(payload=payload)

tests/app/clients/test_cbc_proxy.py

@@ -2,13 +2,24 @@ import json
 import uuid
 from collections import namedtuple
 from datetime import datetime
-from unittest.mock import Mock
+from unittest.mock import Mock, call
 
 import pytest
 
 from app.clients.cbc_proxy import CBCProxyClient, CBCProxyException, CBCProxyEE, CBCProxyCanary
 from app.utils import DATETIME_FORMAT
 
+EXAMPLE_AREAS = [{
+    'description': 'london',
+    'polygon': [
+        [51.12, -1.2],
+        [51.12, 1.2],
+        [51.74, 1.2],
+        [51.74, -1.2],
+        [51.12, -1.2],
+    ],
+}]
+
 
 @pytest.fixture(scope='function')
 def cbc_proxy_client(client, mocker):
@@ -75,18 +86,6 @@ def test_cbc_proxy_ee_create_and_send_invokes_function(
     sent = 'a-passed-through-sent-value'
     expires = 'a-passed-through-expires-value'
 
-    # a single area which is a square including london
-    areas = [{
-        'description': 'london',
-        'polygon': [
-            [51.12, -1.2],
-            [51.12, 1.2],
-            [51.74, 1.2],
-            [51.74, -1.2],
-            [51.12, -1.2],
-        ],
-    }]
-
     ld_client_mock = mocker.patch.object(
         cbc_proxy_ee,
         '_lambda_client',
@@ -102,7 +101,7 @@ def test_cbc_proxy_ee_create_and_send_invokes_function(
         message_number='0000007b',
         headline=headline,
         description=description,
-        areas=areas,
+        areas=EXAMPLE_AREAS,
         sent=sent, expires=expires,
     )
 
@@ -122,7 +121,7 @@ def test_cbc_proxy_ee_create_and_send_invokes_function(
     assert payload['message_type'] == 'alert'
     assert payload['headline'] == headline
     assert payload['description'] == description
-    assert payload['areas'] == areas
+    assert payload['areas'] == EXAMPLE_AREAS
     assert payload['sent'] == sent
     assert payload['expires'] == expires
     assert payload['language'] == expected_language
@@ -200,18 +199,6 @@ def test_cbc_proxy_vodafone_create_and_send_invokes_function(
     sent = 'a-passed-through-sent-value'
     expires = 'a-passed-through-expires-value'
 
-    # a single area which is a square including london
-    areas = [{
-        'description': 'london',
-        'polygon': [
-            [51.12, -1.2],
-            [51.12, 1.2],
-            [51.74, 1.2],
-            [51.74, -1.2],
-            [51.12, -1.2],
-        ],
-    }]
-
     ld_client_mock = mocker.patch.object(
         cbc_proxy_vodafone,
         '_lambda_client',
@@ -227,7 +214,7 @@ def test_cbc_proxy_vodafone_create_and_send_invokes_function(
         message_number='0000007b',
         headline=headline,
         description=description,
-        areas=areas,
+        areas=EXAMPLE_AREAS,
         sent=sent, expires=expires,
     )
 
@@ -247,7 +234,7 @@ def test_cbc_proxy_vodafone_create_and_send_invokes_function(
     assert payload['message_type'] == 'alert'
     assert payload['headline'] == headline
     assert payload['description'] == description
-    assert payload['areas'] == areas
+    assert payload['areas'] == EXAMPLE_AREAS
     assert payload['sent'] == sent
     assert payload['expires'] == expires
     assert payload['language'] == expected_language
@@ -312,28 +299,115 @@ def test_cbc_proxy_vodafone_cancel_invokes_function(mocker, cbc_proxy_vodafone):
     assert payload['sent'] == sent
 
 
-def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy_ee):
-    identifier = 'my-identifier'
-    headline = 'my-headline'
-    description = 'my-description'
-
-    sent = 'a-passed-through-sent-value'
-    expires = 'a-passed-through-expires-value'
-
-    # a single area which is a square including london
-    areas = [{
-        'description': 'london',
-        'polygon': [
-            [51.12, -1.2],
-            [51.12, 1.2],
-            [51.74, 1.2],
-            [51.74, -1.2],
-            [51.12, -1.2],
-        ],
-    }]
+@pytest.mark.parametrize('cbc', ['bt-ee', 'vodafone'])
+def test_cbc_proxy_will_failover_to_second_lambda_if_function_error(
+    mocker,
+    cbc_proxy_ee,
+    cbc_proxy_vodafone,
+    cbc
+):
+    cbc_proxy = cbc_proxy_ee if cbc == 'bt-ee' else cbc_proxy_vodafone
 
     ld_client_mock = mocker.patch.object(
-        cbc_proxy_ee,
+        cbc_proxy,
+        '_lambda_client',
+        create=True,
+    )
+
+    ld_client_mock.invoke.side_effect = [
+        {
+            'StatusCode': 200,
+            'FunctionError': 'Handled',
+            'Payload': {
+                "errorMessage": "",
+                "errorType": "CBCNewConnectionError"
+            }
+        },
+        {
+            'StatusCode': 200
+        }
+    ]
+
+    cbc_proxy.create_and_send_broadcast(
+        identifier='my-identifier',
+        message_number='0000007b',
+        headline='my-headline',
+        description='test-description',
+        areas=EXAMPLE_AREAS,
+        sent='a-passed-through-sent-value',
+        expires='a-passed-through-expires-value',
+    )
+
+    assert ld_client_mock.invoke.call_args_list == [
+        call(
+            FunctionName=f'{cbc}-1-proxy',
+            InvocationType='RequestResponse',
+            Payload=mocker.ANY,
+        ),
+        call(
+            FunctionName=f'{cbc}-2-proxy',
+            InvocationType='RequestResponse',
+            Payload=mocker.ANY,
+        )
+    ]
+
+
+@pytest.mark.parametrize('cbc', ['bt-ee', 'vodafone'])
+def test_cbc_proxy_will_failover_to_second_lambda_if_invoke_error(
+    mocker,
+    cbc_proxy_ee,
+    cbc_proxy_vodafone,
+    cbc
+):
+    cbc_proxy = cbc_proxy_ee if cbc == 'bt-ee' else cbc_proxy_vodafone
+
+    ld_client_mock = mocker.patch.object(
+        cbc_proxy,
+        '_lambda_client',
+        create=True,
+    )
+
+    ld_client_mock.invoke.side_effect = [
+        {
+            'StatusCode': 400
+        },
+        {
+            'StatusCode': 200
+        }
+    ]
+
+    cbc_proxy.create_and_send_broadcast(
+        identifier='my-identifier',
+        message_number='0000007b',
+        headline='my-headline',
+        description='test-description',
+        areas=EXAMPLE_AREAS,
+        sent='a-passed-through-sent-value',
+        expires='a-passed-through-expires-value',
+    )
+
+    assert ld_client_mock.invoke.call_args_list == [
+        call(
+            FunctionName=f'{cbc}-1-proxy',
+            InvocationType='RequestResponse',
+            Payload=mocker.ANY,
+        ),
+        call(
+            FunctionName=f'{cbc}-2-proxy',
+            InvocationType='RequestResponse',
+            Payload=mocker.ANY,
+        )
+    ]
+
+
+@pytest.mark.parametrize('cbc', ['bt-ee', 'vodafone'])
+def test_cbc_proxy_create_and_send_tries_failover_lambda_on_invoke_error_and_raises_if_both_invoke_error(
+    mocker, cbc_proxy_ee, cbc_proxy_vodafone, cbc
+):
+    cbc_proxy = cbc_proxy_ee if cbc == 'bt-ee' else cbc_proxy_vodafone
+
+    ld_client_mock = mocker.patch.object(
+        cbc_proxy,
         '_lambda_client',
         create=True,
     )
@@ -343,46 +417,40 @@ def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy_ee):
     }
 
     with pytest.raises(CBCProxyException) as e:
-        cbc_proxy_ee.create_and_send_broadcast(
-            identifier=identifier,
+        cbc_proxy.create_and_send_broadcast(
+            identifier='my-identifier',
             message_number='0000007b',
-            headline=headline,
-            description=description,
-            areas=areas,
-            sent=sent, expires=expires,
+            headline='my-headline',
+            description='my-description',
+            areas=EXAMPLE_AREAS,
+            sent='a-passed-through-sent-value',
+            expires='a-passed-through-expires-value',
         )
 
-    assert e.match('Could not invoke lambda')
+    assert e.match(f'Lambda failed for both {cbc}-1-proxy and {cbc}-2-proxy')
 
-    ld_client_mock.invoke.assert_called_once_with(
-        FunctionName='bt-ee-1-proxy',
-        InvocationType='RequestResponse',
-        Payload=mocker.ANY,
-    )
+    assert ld_client_mock.invoke.call_args_list == [
+        call(
+            FunctionName=f'{cbc}-1-proxy',
+            InvocationType='RequestResponse',
+            Payload=mocker.ANY,
+        ),
+        call(
+            FunctionName=f'{cbc}-2-proxy',
+            InvocationType='RequestResponse',
+            Payload=mocker.ANY,
+        )
+    ]
 
 
-def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy_ee):
-    identifier = 'my-identifier'
-    headline = 'my-headline'
-    description = 'my-description'
-
-    sent = 'a-passed-through-sent-value'
-    expires = 'a-passed-through-expires-value'
-
-    # a single area which is a square including london
-    areas = [{
-        'description': 'london',
-        'polygon': [
-            [51.12, -1.2],
-            [51.12, 1.2],
-            [51.74, 1.2],
-            [51.74, -1.2],
-            [51.12, -1.2],
-        ],
-    }]
+@pytest.mark.parametrize('cbc', ['bt-ee', 'vodafone'])
+def test_cbc_proxy_create_and_send_tries_failover_lambda_on_function_error_and_raises_if_both_function_error(
+    mocker, cbc_proxy_ee, cbc_proxy_vodafone, cbc
+):
+    cbc_proxy = cbc_proxy_ee if cbc == 'bt-ee' else cbc_proxy_vodafone
 
     ld_client_mock = mocker.patch.object(
-        cbc_proxy_ee,
+        cbc_proxy,
         '_lambda_client',
         create=True,
     )
@@ -390,25 +458,37 @@ def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy_ee):
     ld_client_mock.invoke.return_value = {
         'StatusCode': 200,
         'FunctionError': 'something',
+        'Payload': {
+            'errorMessage': 'some message',
+            'errorType': 'SomeErrorType'
+        }
     }
 
     with pytest.raises(CBCProxyException) as e:
-        cbc_proxy_ee.create_and_send_broadcast(
-            identifier=identifier,
+        cbc_proxy.create_and_send_broadcast(
+            identifier='my-identifier',
             message_number='0000007b',
-            headline=headline,
-            description=description,
-            areas=areas,
-            sent=sent, expires=expires,
+            headline='my-headline',
+            description='my-description',
+            areas=EXAMPLE_AREAS,
+            sent='a-passed-through-sent-value',
+            expires='a-passed-through-expires-value',
         )
 
-    assert e.match('Function exited with unhandled exception')
+    assert e.match(f'Lambda failed for both {cbc}-1-proxy and {cbc}-2-proxy')
 
-    ld_client_mock.invoke.assert_called_once_with(
-        FunctionName='bt-ee-1-proxy',
-        InvocationType='RequestResponse',
-        Payload=mocker.ANY,
-    )
+    assert ld_client_mock.invoke.call_args_list == [
+        call(
+            FunctionName=f'{cbc}-1-proxy',
+            InvocationType='RequestResponse',
+            Payload=mocker.ANY,
+        ),
+        call(
+            FunctionName=f'{cbc}-2-proxy',
+            InvocationType='RequestResponse',
+            Payload=mocker.ANY,
+        )
+    ]
 
 
 def test_cbc_proxy_send_canary_invokes_function(mocker, cbc_proxy_client):