diff --git a/app/clients/sms/__init__.py b/app/clients/sms/__init__.py index 3a2ba1367..0aca5b58c 100644 --- a/app/clients/sms/__init__.py +++ b/app/clients/sms/__init__.py @@ -12,7 +12,7 @@ class SmsClientResponseException(ClientException): self.message = message def __str__(self): - return "Message {}".format(self.message) + return f"SMS client error ({self.message})" class SmsClient(Client): diff --git a/app/clients/sms/firetext.py b/app/clients/sms/firetext.py index f2336f2aa..0d8f5a685 100644 --- a/app/clients/sms/firetext.py +++ b/app/clients/sms/firetext.py @@ -44,18 +44,6 @@ def get_message_status_and_reason_from_firetext_code(detailed_status_code): return firetext_codes[detailed_status_code]['status'], firetext_codes[detailed_status_code]['reason'] -class FiretextClientResponseException(SmsClientResponseException): - def __init__(self, response, exception): - status_code = response.status_code if response is not None else 504 - text = response.text if response is not None else "Gateway Time-out" - self.status_code = status_code - self.text = text - self.exception = exception - - def __str__(self): - return "Code {} text {} exception {}".format(self.status_code, self.text, str(self.exception)) - - class FiretextClient(SmsClient): ''' FireText sms client. @@ -91,10 +79,10 @@ class FiretextClient(SmsClient): try: json.loads(response.text) if response.json()['code'] != 0: - raise ValueError() - except (ValueError, AttributeError) as e: - raise FiretextClientResponseException(response=response, exception=e) - except RequestException as e: - raise FiretextClientResponseException(response=e.response, exception=e) + raise ValueError("Expected 'code' to be '0'") + except (ValueError, AttributeError): + raise SmsClientResponseException("Invalid response JSON") + except RequestException: + raise SmsClientResponseException("Request failed") return response diff --git a/app/clients/sms/mmg.py b/app/clients/sms/mmg.py index a1408a16e..b05c275ea 100644 --- a/app/clients/sms/mmg.py +++ b/app/clients/sms/mmg.py @@ -102,9 +102,9 @@ class MMGClient(SmsClient): response.raise_for_status() try: json.loads(response.text) - except (ValueError, AttributeError) as e: - raise MMGClientResponseException(response=response, exception=e) - except RequestException as e: - raise MMGClientResponseException(response=e.response, exception=e) + except (ValueError, AttributeError): + raise SmsClientResponseException("Invalid response JSON") + except RequestException: + raise SmsClientResponseException("Request failed") return response diff --git a/app/clients/sms/reach.py b/app/clients/sms/reach.py index ad4eee17f..3541f8b5c 100644 --- a/app/clients/sms/reach.py +++ b/app/clients/sms/reach.py @@ -16,19 +16,6 @@ def get_reach_responses(status, detailed_status_code=None): raise KeyError -class ReachClientResponseException(SmsClientResponseException): - def __init__(self, response, exception): - status_code = response.status_code if response is not None else 504 - text = response.text if response is not None else "Gateway Time-out" - - self.status_code = status_code - self.text = text - self.exception = exception - - def __str__(self): - return "Code {} text {} exception {}".format(self.status_code, self.text, str(self.exception)) - - class ReachClient(SmsClient): def init_app(self, *args, **kwargs): super().init_app(*args, **kwargs) @@ -57,9 +44,9 @@ class ReachClient(SmsClient): response.raise_for_status() try: json.loads(response.text) - except (ValueError, AttributeError) as e: - raise ReachClientResponseException(response=response, exception=e) - except RequestException as e: - raise ReachClientResponseException(response=e.response, exception=e) + except (ValueError, AttributeError): + raise SmsClientResponseException("Invalid response JSON") + except RequestException: + raise SmsClientResponseException("Request failed") return response diff --git a/tests/app/clients/test_firetext.py b/tests/app/clients/test_firetext.py index 089a61fc0..75febf605 100644 --- a/tests/app/clients/test_firetext.py +++ b/tests/app/clients/test_firetext.py @@ -2,11 +2,9 @@ from urllib.parse import parse_qs import pytest import requests_mock -from requests import HTTPError from requests.exceptions import ConnectTimeout, ReadTimeout from app.clients.sms.firetext import ( - FiretextClientResponseException, SmsClientResponseException, get_firetext_responses, ) @@ -116,9 +114,7 @@ def test_try_send_sms_raises_if_firetext_rejects(mocker, mock_firetext_client): request_mock.post('https://example.com/firetext', json=response_dict, status_code=200) mock_firetext_client.try_send_sms(to, content, reference, False, 'sender') - assert exc.value.status_code == 200 - assert '"description": "Some kind of error"' in exc.value.text - assert '"code": 1' in exc.value.text + assert "Invalid response JSON" in str(exc.value) def test_try_send_sms_raises_if_firetext_rejects_with_unexpected_data(mocker, mock_firetext_client): @@ -129,9 +125,7 @@ def test_try_send_sms_raises_if_firetext_rejects_with_unexpected_data(mocker, mo request_mock.post('https://example.com/firetext', json=response_dict, status_code=400) mock_firetext_client.try_send_sms(to, content, reference, False, 'sender') - assert exc.value.status_code == 400 - assert exc.value.text == '{"something": "gone bad"}' - assert type(exc.value.exception) == HTTPError + assert "Request failed" in str(exc.value) def test_try_send_sms_raises_if_firetext_fails_to_return_json(notify_api, mock_firetext_client): @@ -142,28 +136,24 @@ def test_try_send_sms_raises_if_firetext_fails_to_return_json(notify_api, mock_f request_mock.post('https://example.com/firetext', text=response_dict, status_code=200) mock_firetext_client.try_send_sms(to, content, reference, False, 'sender') - assert 'Code 200 text NOT AT ALL VALID JSON {"key" : "value"}} exception Expecting value: line 1 column 1 (char 0)' in str(exc.value) # noqa - assert exc.value.status_code == 200 - assert exc.value.text == 'NOT AT ALL VALID JSON {"key" : "value"}}' + assert "Invalid response JSON" in str(exc.value) def test_try_send_sms_raises_if_firetext_rejects_with_connect_timeout(rmock, mock_firetext_client): to = content = reference = 'foo' - with pytest.raises(FiretextClientResponseException) as exc: + with pytest.raises(SmsClientResponseException) as exc: rmock.register_uri('POST', 'https://example.com/firetext', exc=ConnectTimeout) mock_firetext_client.try_send_sms(to, content, reference, False, 'sender') - assert exc.value.status_code == 504 - assert exc.value.text == 'Gateway Time-out' + assert "Request failed" in str(exc.value) def test_try_send_sms_raises_if_firetext_rejects_with_read_timeout(rmock, mock_firetext_client): to = content = reference = 'foo' - with pytest.raises(FiretextClientResponseException) as exc: + with pytest.raises(SmsClientResponseException) as exc: rmock.register_uri('POST', 'https://example.com/firetext', exc=ReadTimeout) mock_firetext_client.try_send_sms(to, content, reference, False, 'sender') - assert exc.value.status_code == 504 - assert exc.value.text == 'Gateway Time-out' + assert "Request failed" in str(exc.value) diff --git a/tests/app/clients/test_mmg.py b/tests/app/clients/test_mmg.py index 0164af452..767701ed9 100644 --- a/tests/app/clients/test_mmg.py +++ b/tests/app/clients/test_mmg.py @@ -1,11 +1,9 @@ import pytest import requests_mock -from requests import HTTPError from requests.exceptions import ConnectTimeout, ReadTimeout from app import mmg_client -from app.clients.sms import SmsClientResponseException -from app.clients.sms.mmg import MMGClientResponseException, get_mmg_responses +from app.clients.sms.mmg import SmsClientResponseException, get_mmg_responses @pytest.mark.parametrize('detailed_status_code, result', [ @@ -85,10 +83,7 @@ def test_try_send_sms_raises_if_mmg_rejects(notify_api, mocker): request_mock.post('https://example.com/mmg', json=response_dict, status_code=400) mmg_client.try_send_sms(to, content, reference, False, 'sender') - assert exc.value.status_code == 400 - assert '"Error": 206' in exc.value.text - assert '"Description": "Some kind of error"' in exc.value.text - assert type(exc.value.exception) == HTTPError + assert "Request failed" in str(exc.value) def test_try_send_sms_raises_if_mmg_fails_to_return_json(notify_api, mocker): @@ -99,28 +94,24 @@ def test_try_send_sms_raises_if_mmg_fails_to_return_json(notify_api, mocker): request_mock.post('https://example.com/mmg', text=response_dict, status_code=200) mmg_client.try_send_sms(to, content, reference, False, 'sender') - assert 'Code 200 text NOT AT ALL VALID JSON {"key" : "value"}} exception Expecting value: line 1 column 1 (char 0)' in str(exc.value) # noqa - assert exc.value.status_code == 200 - assert exc.value.text == 'NOT AT ALL VALID JSON {"key" : "value"}}' + assert "Invalid response JSON" in str(exc.value) def test_try_send_sms_raises_if_mmg_rejects_with_connect_timeout(rmock): to = content = reference = 'foo' - with pytest.raises(MMGClientResponseException) as exc: + with pytest.raises(SmsClientResponseException) as exc: rmock.register_uri('POST', 'https://example.com/mmg', exc=ConnectTimeout) mmg_client.try_send_sms(to, content, reference, False, 'sender') - assert exc.value.status_code == 504 - assert exc.value.text == 'Gateway Time-out' + assert "Request failed" in str(exc.value) def test_try_send_sms_raises_if_mmg_rejects_with_read_timeout(rmock): to = content = reference = 'foo' - with pytest.raises(MMGClientResponseException) as exc: + with pytest.raises(SmsClientResponseException) as exc: rmock.register_uri('POST', 'https://example.com/mmg', exc=ReadTimeout) mmg_client.try_send_sms(to, content, reference, False, 'sender') - assert exc.value.status_code == 504 - assert exc.value.text == 'Gateway Time-out' + assert "Request failed" in str(exc.value) diff --git a/tests/app/clients/test_reach.py b/tests/app/clients/test_reach.py index 7cc8b2bc3..b62a5fd0b 100644 --- a/tests/app/clients/test_reach.py +++ b/tests/app/clients/test_reach.py @@ -1,11 +1,9 @@ import pytest import requests_mock -from requests import HTTPError from requests.exceptions import ConnectTimeout, ReadTimeout from app import reach_client from app.clients.sms import SmsClientResponseException -from app.clients.sms.reach import ReachClientResponseException # TODO: tests for get_reach_responses @@ -52,10 +50,7 @@ def test_try_send_sms_raises_if_reach_rejects(notify_api, mocker): request_mock.post('https://example.com/reach', json=response_dict, status_code=400) reach_client.try_send_sms(to, content, reference, False, 'sender') - assert exc.value.status_code == 400 - assert '"Error": 206' in exc.value.text - assert '"Description": "Some kind of error"' in exc.value.text - assert type(exc.value.exception) == HTTPError + assert "Request failed" in str(exc) def test_try_send_sms_raises_if_reach_fails_to_return_json(notify_api, mocker): @@ -66,28 +61,24 @@ def test_try_send_sms_raises_if_reach_fails_to_return_json(notify_api, mocker): request_mock.post('https://example.com/reach', text=response_dict, status_code=200) reach_client.try_send_sms(to, content, reference, False, 'sender') - assert 'Code 200 text NOT AT ALL VALID JSON {"key" : "value"}} exception Expecting value: line 1 column 1 (char 0)' in str(exc.value) # noqa - assert exc.value.status_code == 200 - assert exc.value.text == 'NOT AT ALL VALID JSON {"key" : "value"}}' + assert 'Invalid response JSON' in str(exc.value) def test_try_send_sms_raises_if_reach_rejects_with_connect_timeout(rmock): to = content = reference = 'foo' - with pytest.raises(ReachClientResponseException) as exc: + with pytest.raises(SmsClientResponseException) as exc: rmock.register_uri('POST', 'https://example.com/reach', exc=ConnectTimeout) reach_client.try_send_sms(to, content, reference, False, 'sender') - assert exc.value.status_code == 504 - assert exc.value.text == 'Gateway Time-out' + assert 'Request failed' in str(exc.value) def test_try_send_sms_raises_if_reach_rejects_with_read_timeout(rmock): to = content = reference = 'foo' - with pytest.raises(ReachClientResponseException) as exc: + with pytest.raises(SmsClientResponseException) as exc: rmock.register_uri('POST', 'https://example.com/reach', exc=ReadTimeout) reach_client.try_send_sms(to, content, reference, False, 'sender') - assert exc.value.status_code == 504 - assert exc.value.text == 'Gateway Time-out' + assert 'Request failed' in str(exc.value)