diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index 8500e57c4..d53c0131b 100644 --- a/app/clients/cbc_proxy.py +++ b/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) diff --git a/tests/app/clients/test_cbc_proxy.py b/tests/app/clients/test_cbc_proxy.py index c8db4cc45..ed5b5a3ca 100644 --- a/tests/app/clients/test_cbc_proxy.py +++ b/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):