Retry with failover lambda for FunctionError and status > 299

For all FunctionErrors, and for invoke errors (status > 299) we
want to retry with failover lambda.

We are doing this, because if there is a connection or other error
with one lambda, the failover lambda may still work and it's
worth trying.

With time, we will probably have more complex retry flow, depending
on the error and even maybe differing for each MNO (broadcast provider).

app/clients/cbc_proxy.py

@@ -112,12 +112,10 @@ class CBCProxyClientBase(ABC):
         payload_bytes = bytes(json.dumps(payload), encoding='utf8')
         result = self._invoke_lambda(self.lambda_name, payload_bytes)
 
-        if 'FunctionError' in result:
+        if not result:
-            if result['Payload']['errorType'] == "CBCNewConnectionError":
+            failover_result = self._invoke_lambda(self.failover_lambda_name, payload_bytes)
-                current_app.logger.info(f"Got CBCNewConnectionError for {self.lambda_name}, calling failover lambda")
+            if not failover_result:
-                result = self._invoke_lambda(self.failover_lambda_name, payload_bytes)
+                raise CBCProxyException(f'Lambda failed for both {self.lambda_name} and {self.failover_lambda_name}')
-            else:
-                raise CBCProxyException('Function exited with unhandled exception')
 
         return result
 
@@ -128,12 +126,24 @@ class CBCProxyClientBase(ABC):
             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
 
-        current_app.logger.info(f"Finished calling lambda {lambda_name}")
+        elif 'FunctionError' in result:
-        return result
+            current_app.logger.info(
+                f"Error calling lambda {self.lambda_name} with function error { result['Payload'] }"
+            )
+            success = False
+
+        else:
+            success = True
+
+        return success
 
     def infer_language_from(self, content):
         if non_gsm_characters(content):

tests/app/clients/test_cbc_proxy.py

@@ -127,60 +127,6 @@ def test_cbc_proxy_ee_create_and_send_invokes_function(
     assert payload['language'] == expected_language
 
 
-
-@pytest.mark.parametrize('cbc', ['bt-ee', 'vodafone'])
-def test_cbc_proxy_will_failover_to_second_lambda_if_connection_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': 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,
-        )
-    ]
-
-
 def test_cbc_proxy_ee_cancel_invokes_function(mocker, cbc_proxy_ee):
     identifier = 'my-identifier'
     MockProviderMessage = namedtuple(
@@ -353,16 +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):
+@pytest.mark.parametrize('cbc', ['bt-ee', 'vodafone'])
-    identifier = 'my-identifier'
+def test_cbc_proxy_will_failover_to_second_lambda_if_function_error(
-    headline = 'my-headline'
+    mocker,
-    description = 'my-description'
+    cbc_proxy_ee,
-
+    cbc_proxy_vodafone,
-    sent = 'a-passed-through-sent-value'
+    cbc
-    expires = 'a-passed-through-expires-value'
+):
+    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,
     )
@@ -372,34 +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(
+        cbc_proxy.create_and_send_broadcast(
-            identifier=identifier,
+            identifier='my-identifier',
             message_number='0000007b',
-            headline=headline,
+            headline='my-headline',
-            description=description,
+            description='my-description',
             areas=EXAMPLE_AREAS,
-            sent=sent, expires=expires,
+            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(
+    assert ld_client_mock.invoke.call_args_list == [
-        FunctionName='bt-ee-1-proxy',
+        call(
-        InvocationType='RequestResponse',
+            FunctionName=f'{cbc}-1-proxy',
-        Payload=mocker.ANY,
+            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):
+@pytest.mark.parametrize('cbc', ['bt-ee', 'vodafone'])
-    identifier = 'my-identifier'
+def test_cbc_proxy_create_and_send_tries_failover_lambda_on_function_error_and_raises_if_both_function_error(
-    headline = 'my-headline'
+    mocker, cbc_proxy_ee, cbc_proxy_vodafone, cbc
-    description = 'my-description'
+):
-
+    cbc_proxy = cbc_proxy_ee if cbc == 'bt-ee' else cbc_proxy_vodafone
-    sent = 'a-passed-through-sent-value'
-    expires = 'a-passed-through-expires-value'
 
     ld_client_mock = mocker.patch.object(
-        cbc_proxy_ee,
+        cbc_proxy,
         '_lambda_client',
         create=True,
     )
@@ -414,22 +465,30 @@ def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy_ee):
     }
 
     with pytest.raises(CBCProxyException) as e:
-        cbc_proxy_ee.create_and_send_broadcast(
+        cbc_proxy.create_and_send_broadcast(
-            identifier=identifier,
+            identifier='my-identifier',
             message_number='0000007b',
-            headline=headline,
+            headline='my-headline',
-            description=description,
+            description='my-description',
             areas=EXAMPLE_AREAS,
-            sent=sent, expires=expires,
+            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(
+    assert ld_client_mock.invoke.call_args_list == [
-        FunctionName='bt-ee-1-proxy',
+        call(
-        InvocationType='RequestResponse',
+            FunctionName=f'{cbc}-1-proxy',
-        Payload=mocker.ANY,
+            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):