From 47c80c5e6a4b896900f8902eabf31756bc579c16 Mon Sep 17 00:00:00 2001 From: Pranjal Mittal Date: Thu, 17 Jan 2019 15:10:20 -0800 Subject: [PATCH 1/4] Trickle up exceptions --- braze/client.py | 40 +++++++++++++++++++++----------------- tests/braze/test_client.py | 39 ++++++++++++++++++++----------------- 2 files changed, 43 insertions(+), 36 deletions(-) diff --git a/braze/client.py b/braze/client.py index f07d99c..bae30b5 100644 --- a/braze/client.py +++ b/braze/client.py @@ -22,10 +22,14 @@ def __init__(self, reset_epoch_s): :param float reset_epoch_s: Unix timestamp for when the API may be called again. """ self.reset_epoch_s = reset_epoch_s - super(BrazeRateLimitError, self).__init__("BrazeRateLimitError") + super(BrazeRateLimitError, self).__init__() -class BrazeInternalServerError(Exception): +class BrazeClientError(Exception): + pass + + +class BrazeInternalServerError(BrazeClientError): pass @@ -131,21 +135,21 @@ def __create_request(self, payload): payload["api_key"] = self.api_key response = {"errors": []} - try: - r = self._post_request_with_retries(payload) - response.update(r.json()) - response["status_code"] = r.status_code - - message = response["message"] - if message == "success" or message == "queued": - if not response["errors"]: - response["success"] = True - else: - # Non-Fatal errors - pass - - except (RequestException, BrazeRateLimitError, BrazeInternalServerError) as e: - response["errors"].append(str(e)) + r = self._post_request_with_retries(payload) + response.update(r.json()) + response["status_code"] = r.status_code + + message = response["message"] + if message == "success" or message == "queued": + if not response["errors"]: + response["success"] = True + else: + # Non-Fatal errors + pass + + if message != "success": + # message contains the fatal error message from Braze + raise BrazeClientError(message, response["errors"]) if "success" not in response: response["success"] = False @@ -174,5 +178,5 @@ def _post_request_with_retries(self, payload): reset_epoch_s = float(r.headers.get("X-RateLimit-Reset", 0)) raise BrazeRateLimitError(reset_epoch_s) elif str(r.status_code).startswith("5"): - raise BrazeInternalServerError("BrazeInternalServerError") + raise BrazeInternalServerError return r diff --git a/tests/braze/test_client.py b/tests/braze/test_client.py index fb4c601..78d54bd 100644 --- a/tests/braze/test_client.py +++ b/tests/braze/test_client.py @@ -1,6 +1,7 @@ import time -from braze.client import _wait_random_exp_or_rate_limit +from braze.client import _wait_random_exp_or_rate_limit, BrazeClientError, \ + BrazeInternalServerError from braze.client import BrazeClient from braze.client import BrazeRateLimitError from braze.client import MAX_RETRIES @@ -93,22 +94,24 @@ def test_user_track( def test_user_track_request_exception( self, braze_client, mocker, attributes, events, purchases ): - error_msg = "RequestException Error Message" mocker.patch.object( BrazeClient, "_post_request_with_retries", - side_effect=RequestException(error_msg), + side_effect=RequestException, ) - response = braze_client.user_track( - attributes=attributes, events=events, purchases=purchases - ) + with pytest.raises(RequestException): + braze_client.user_track( + attributes=attributes, events=events, purchases=purchases + ) + assert braze_client.api_url + "/users/track" == braze_client.request_url - assert response["status_code"] == 0 - assert error_msg in response["errors"] @pytest.mark.parametrize( - "status_code, retry_attempts", [(500, MAX_RETRIES), (401, 1)] + "status_code, retry_attempts, error", + [ + (500, MAX_RETRIES, BrazeInternalServerError), + (401, 1, BrazeClientError)] ) def test_retries_for_errors( self, @@ -119,6 +122,7 @@ def test_retries_for_errors( attributes, events, purchases, + error, ): headers = {"Content-Type": "application/json"} error_msg = "Internal Server Error" @@ -127,13 +131,13 @@ def test_retries_for_errors( ANY, json=mock_json, status_code=status_code, headers=headers ) - response = braze_client.user_track( - attributes=attributes, events=events, purchases=purchases - ) + with pytest.raises(error): + braze_client.user_track( + attributes=attributes, events=events, purchases=purchases + ) stats = braze_client._post_request_with_retries.retry.statistics assert stats["attempt_number"] == retry_attempts - assert response["success"] is False @freeze_time() @pytest.mark.parametrize( @@ -159,14 +163,13 @@ def test_retries_for_rate_limit_errors( mock_json = {"message": error_msg, "errors": error_msg} requests_mock.post(ANY, json=mock_json, status_code=429, headers=headers) - response = braze_client.user_track( - attributes=attributes, events=events, purchases=purchases - ) + with pytest.raises(BrazeRateLimitError): + braze_client.user_track( + attributes=attributes, events=events, purchases=purchases + ) stats = braze_client._post_request_with_retries.retry.statistics assert stats["attempt_number"] == expected_attempts - assert response["success"] is False - assert "BrazeRateLimitError" in response["errors"] # Ensure the correct wait time is used when rate limited for i in range(expected_attempts - 1): From a67665520995bce708b4ef6881a0b20253b6144c Mon Sep 17 00:00:00 2001 From: Pranjal Mittal Date: Thu, 17 Jan 2019 15:12:39 -0800 Subject: [PATCH 2/4] fix style/format --- braze/client.py | 1 - tests/braze/test_client.py | 13 +++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/braze/client.py b/braze/client.py index bae30b5..91e0380 100644 --- a/braze/client.py +++ b/braze/client.py @@ -1,7 +1,6 @@ import time import requests -from requests.exceptions import RequestException from tenacity import retry from tenacity import stop_after_attempt from tenacity import wait_random_exponential diff --git a/tests/braze/test_client.py b/tests/braze/test_client.py index 78d54bd..f5871b0 100644 --- a/tests/braze/test_client.py +++ b/tests/braze/test_client.py @@ -1,8 +1,9 @@ import time -from braze.client import _wait_random_exp_or_rate_limit, BrazeClientError, \ - BrazeInternalServerError +from braze.client import _wait_random_exp_or_rate_limit from braze.client import BrazeClient +from braze.client import BrazeClientError +from braze.client import BrazeInternalServerError from braze.client import BrazeRateLimitError from braze.client import MAX_RETRIES from braze.client import MAX_WAIT_SECONDS @@ -95,9 +96,7 @@ def test_user_track_request_exception( self, braze_client, mocker, attributes, events, purchases ): mocker.patch.object( - BrazeClient, - "_post_request_with_retries", - side_effect=RequestException, + BrazeClient, "_post_request_with_retries", side_effect=RequestException ) with pytest.raises(RequestException): @@ -109,9 +108,7 @@ def test_user_track_request_exception( @pytest.mark.parametrize( "status_code, retry_attempts, error", - [ - (500, MAX_RETRIES, BrazeInternalServerError), - (401, 1, BrazeClientError)] + [(500, MAX_RETRIES, BrazeInternalServerError), (401, 1, BrazeClientError)], ) def test_retries_for_errors( self, From 00851d079901fbaa5acf0b03d6f16eb212bed837 Mon Sep 17 00:00:00 2001 From: Pranjal Mittal Date: Thu, 17 Jan 2019 15:21:31 -0800 Subject: [PATCH 3/4] docstrings --- braze/client.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/braze/client.py b/braze/client.py index 91e0380..360cbc3 100644 --- a/braze/client.py +++ b/braze/client.py @@ -25,10 +25,21 @@ def __init__(self, reset_epoch_s): class BrazeClientError(Exception): + """ + Represents any Braze Fatal Error. + + https://www.braze.com/docs/developer_guide/rest_api/user_data/#user-track-responses + """ + pass class BrazeInternalServerError(BrazeClientError): + """ + Used for Braze API responses where response code is of type 5XX suggesting + Braze side server errors. + """ + pass From 4408c52ae704bb63483e166629976f6d92058046 Mon Sep 17 00:00:00 2001 From: Pranjal Mittal Date: Thu, 24 Jan 2019 15:53:07 -0800 Subject: [PATCH 4/4] simplify response success setting --- braze/client.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/braze/client.py b/braze/client.py index 360cbc3..bc71a31 100644 --- a/braze/client.py +++ b/braze/client.py @@ -150,20 +150,14 @@ def __create_request(self, payload): response["status_code"] = r.status_code message = response["message"] - if message == "success" or message == "queued": - if not response["errors"]: - response["success"] = True - else: - # Non-Fatal errors - pass + response["success"] = ( + message in ("success", "queued") and not response["errors"] + ) if message != "success": # message contains the fatal error message from Braze raise BrazeClientError(message, response["errors"]) - if "success" not in response: - response["success"] = False - if "status_code" not in response: response["status_code"] = 0