From 18889512d0e41985a9a77ee22296fc2c52ab0b2e Mon Sep 17 00:00:00 2001 From: Francis Devereux Date: Thu, 21 Jan 2016 11:10:35 +0000 Subject: [PATCH 01/16] Include complete error data in HubspotClientError Including all error information returned by HubSpot aids troubleshooting and allows better error reporting to the user. --- hubspot/connection/__init__.py | 1 + hubspot/connection/exc.py | 3 ++- tests/test_connection.py | 21 ++++++++++++++++++--- tests/test_testing_utils.py | 2 +- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/hubspot/connection/__init__.py b/hubspot/connection/__init__.py index f43f40b..8a3f6d9 100644 --- a/hubspot/connection/__init__.py +++ b/hubspot/connection/__init__.py @@ -204,6 +204,7 @@ def _require_successful_response(response): raise exception_class( error_data['message'], error_data['requestId'], + error_data, ) elif 500 <= response.status_code < 600: raise HubspotServerError(response.reason, response.status_code) diff --git a/hubspot/connection/exc.py b/hubspot/connection/exc.py index 2732fac..4d5352b 100644 --- a/hubspot/connection/exc.py +++ b/hubspot/connection/exc.py @@ -32,10 +32,11 @@ class HubspotClientError(HubspotException): :param unicode request_id: """ - def __init__(self, msg, request_id): + def __init__(self, msg, request_id, error_data): super(HubspotClientError, self).__init__(msg) self.request_id = request_id + self.error_data = error_data class HubspotAuthenticationError(HubspotClientError): diff --git a/tests/test_connection.py b/tests/test_connection.py index b1d6890..b33a9fa 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -244,12 +244,25 @@ def test_server_error_response(self): def test_client_error_response(self): request_id = get_uuid4_str() - error_message = 'Json node is missing child property' + error_message = 'Errors found processing batch update' + failure_messages = [ + { + 'index': 0, + 'error': { + 'status': 'error', + 'message': 'Email address is invalid' + } + } + ] + invalid_emails = [''] body_deserialization = { 'status': 'error', 'message': error_message, - 'requestId': request_id, - } + 'correlationId': '2ebc27ce-cc2d-4b81-99f3-01aa96e05206', + 'invalidEmails': invalid_emails, + 'failureMessages': failure_messages, + 'requestId': request_id + } response_data_maker = _ResponseMaker(400, body_deserialization) connection = _MockPortalConnection(response_data_maker) @@ -259,6 +272,8 @@ def test_client_error_response(self): exception = context_manager.exception eq_(request_id, exception.request_id) eq_(error_message, str(exception)) + eq_(invalid_emails, exception.error_data['invalidEmails']) + eq_(failure_messages, exception.error_data['failureMessages']) class TestAuthentication(object): diff --git a/tests/test_testing_utils.py b/tests/test_testing_utils.py index 3d42928..5acf837 100644 --- a/tests/test_testing_utils.py +++ b/tests/test_testing_utils.py @@ -171,7 +171,7 @@ def test_multiple_api_calls(self): def test_unsuccessful_api_call(self): exception = \ - HubspotAuthenticationError('Must authenticate', get_uuid4_str()) + HubspotAuthenticationError('Must authenticate', get_uuid4_str(), {}) expected_api_call = UnsuccessfulAPICall( _STUB_URL_PATH, 'GET', From 1d49195060e6528a1ccbe4fd6953a5ea1513bf5a Mon Sep 17 00:00:00 2001 From: Francis Devereux Date: Thu, 21 Jan 2016 11:28:58 +0000 Subject: [PATCH 02/16] Include failureMessages in HubspotClientError message (if present) --- hubspot/connection/exc.py | 2 ++ tests/test_connection.py | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/hubspot/connection/exc.py b/hubspot/connection/exc.py index 4d5352b..b13c55c 100644 --- a/hubspot/connection/exc.py +++ b/hubspot/connection/exc.py @@ -33,6 +33,8 @@ class HubspotClientError(HubspotException): """ def __init__(self, msg, request_id, error_data): + if error_data and 'failureMessages' in error_data: + msg = u'%s (%s)' % (msg, error_data['failureMessages']) super(HubspotClientError, self).__init__(msg) self.request_id = request_id diff --git a/tests/test_connection.py b/tests/test_connection.py index b33a9fa..725efe2 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -271,10 +271,23 @@ def test_client_error_response(self): exception = context_manager.exception eq_(request_id, exception.request_id) - eq_(error_message, str(exception)) + eq_("Errors found processing batch update ([{u'index': 0, u'error': {u'status': u'error', u'message': u'Email address is invalid'}}])", + str(exception)) eq_(invalid_emails, exception.error_data['invalidEmails']) eq_(failure_messages, exception.error_data['failureMessages']) + def test_str_client_exception_when_no_failure_messages(self): + request_id = get_uuid4_str() + error_message = 'Json node is missing child property' + error_data = { + 'status': 'error', + 'message': error_message, + 'requestId': request_id, + } + + exception = HubspotClientError(error_message, request_id, error_data) + eq_(error_message, str(exception)) + class TestAuthentication(object): From b7d94ed6b509050c8914d46ade9601ed204636da Mon Sep 17 00:00:00 2001 From: Francis Devereux Date: Thu, 21 Jan 2016 11:30:08 +0000 Subject: [PATCH 03/16] Avoid mutating argument --- hubspot/connection/exc.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hubspot/connection/exc.py b/hubspot/connection/exc.py index b13c55c..36224f0 100644 --- a/hubspot/connection/exc.py +++ b/hubspot/connection/exc.py @@ -32,9 +32,11 @@ class HubspotClientError(HubspotException): :param unicode request_id: """ - def __init__(self, msg, request_id, error_data): + def __init__(self, error_message, request_id, error_data): if error_data and 'failureMessages' in error_data: - msg = u'%s (%s)' % (msg, error_data['failureMessages']) + msg = u'%s (%s)' % (error_message, error_data['failureMessages']) + else: + msg = error_message super(HubspotClientError, self).__init__(msg) self.request_id = request_id From 68c0482c03324e1e21f6992e6f65708944db4007 Mon Sep 17 00:00:00 2001 From: Francis Devereux Date: Thu, 21 Jan 2016 11:45:12 +0000 Subject: [PATCH 04/16] Include the plain error_message in HubspotClientError This is for use by code that wants to show a simple error message to the user, without technical details. Exception.message still contains the technical details (i.e. failureMessages) so that generic exception logging code will log the details needed for diagnosing the failure. --- hubspot/connection/exc.py | 1 + tests/test_connection.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/hubspot/connection/exc.py b/hubspot/connection/exc.py index 36224f0..1ed1f77 100644 --- a/hubspot/connection/exc.py +++ b/hubspot/connection/exc.py @@ -39,6 +39,7 @@ def __init__(self, error_message, request_id, error_data): msg = error_message super(HubspotClientError, self).__init__(msg) + self.error_message = error_message self.request_id = request_id self.error_data = error_data diff --git a/tests/test_connection.py b/tests/test_connection.py index 725efe2..489ab61 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -273,6 +273,7 @@ def test_client_error_response(self): eq_(request_id, exception.request_id) eq_("Errors found processing batch update ([{u'index': 0, u'error': {u'status': u'error', u'message': u'Email address is invalid'}}])", str(exception)) + eq_(error_message, exception.error_message) eq_(invalid_emails, exception.error_data['invalidEmails']) eq_(failure_messages, exception.error_data['failureMessages']) @@ -287,6 +288,7 @@ def test_str_client_exception_when_no_failure_messages(self): exception = HubspotClientError(error_message, request_id, error_data) eq_(error_message, str(exception)) + eq_(error_message, exception.error_message) class TestAuthentication(object): From 501cd7cb4d3eaf4c9f81e4d53bc258f27d432f56 Mon Sep 17 00:00:00 2001 From: Lloyd Nye Date: Mon, 25 Jan 2016 10:39:22 +0000 Subject: [PATCH 05/16] Improve error message --- hubspot/connection/testing.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hubspot/connection/testing.py b/hubspot/connection/testing.py index dfc1379..8ab5175 100644 --- a/hubspot/connection/testing.py +++ b/hubspot/connection/testing.py @@ -64,9 +64,8 @@ def __exit__(self, exc_type, exc_value, traceback): return expected_api_call_count = len(self._expected_api_calls) - pending_api_call_count = expected_api_call_count - self._request_count error_message = \ - '{} more requests were expected'.format(pending_api_call_count) + '{} requests were expected, but only {} calls were received'.format(expected_api_call_count, self._request_count) assert expected_api_call_count == self._request_count, error_message def send_get_request(self, url_path, query_string_args=None): From 5a53d7d7f0ae4bc7a249274e98af090b9c42b20a Mon Sep 17 00:00:00 2001 From: Lloyd Nye Date: Mon, 25 Jan 2016 14:08:50 +0000 Subject: [PATCH 06/16] Restore previous interface. hubspot-connection relies on the existing interface. --- hubspot/connection/exc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hubspot/connection/exc.py b/hubspot/connection/exc.py index 1ed1f77..f3f42b9 100644 --- a/hubspot/connection/exc.py +++ b/hubspot/connection/exc.py @@ -32,7 +32,7 @@ class HubspotClientError(HubspotException): :param unicode request_id: """ - def __init__(self, error_message, request_id, error_data): + def __init__(self, error_message, request_id, error_data=None): if error_data and 'failureMessages' in error_data: msg = u'%s (%s)' % (error_message, error_data['failureMessages']) else: From a3ce40c67a685cca4e057826692f1f0e9bc0b9a6 Mon Sep 17 00:00:00 2001 From: Lloyd Nye Date: Mon, 25 Jan 2016 16:32:04 +0000 Subject: [PATCH 07/16] Fixing test failure --- tests/test_testing_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_testing_utils.py b/tests/test_testing_utils.py index 5acf837..e735401 100644 --- a/tests/test_testing_utils.py +++ b/tests/test_testing_utils.py @@ -189,7 +189,7 @@ def test_unsuccessful_api_call(self): def test_too_few_requests(self): connection = \ self._make_connection_for_expected_api_call(_STUB_API_CALL_1) - error_message = '1 more requests were expected' + error_message = '1 requests were expected, but only 0 calls were received' with assert_raises_substring(AssertionError, error_message): with connection: # Do not make any requests in the connection From 4e92c07f48e9afcb2c12ead26fcd65cd4b210c04 Mon Sep 17 00:00:00 2001 From: Lloyd Nye Date: Tue, 26 Jan 2016 13:54:27 +0000 Subject: [PATCH 08/16] Add doc for new param --- hubspot/connection/exc.py | 1 + 1 file changed, 1 insertion(+) diff --git a/hubspot/connection/exc.py b/hubspot/connection/exc.py index f3f42b9..1bf14c0 100644 --- a/hubspot/connection/exc.py +++ b/hubspot/connection/exc.py @@ -30,6 +30,7 @@ class HubspotClientError(HubspotException): of 40X, except 401 :param unicode request_id: + :param dict optional error_data: """ def __init__(self, error_message, request_id, error_data=None): From 4c97e4df4fdb22fa4a4f2679d453468fb8a2bd9d Mon Sep 17 00:00:00 2001 From: Lloyd Nye Date: Tue, 26 Jan 2016 13:59:24 +0000 Subject: [PATCH 09/16] Add http status code to HubspotClientError creation. The status code is significant in indicating the reason for failure. This is neccesary for clients to act appropriately depending on the failure. --- hubspot/connection/__init__.py | 1 + hubspot/connection/exc.py | 4 +++- tests/test_connection.py | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/hubspot/connection/__init__.py b/hubspot/connection/__init__.py index 8a3f6d9..de369a1 100644 --- a/hubspot/connection/__init__.py +++ b/hubspot/connection/__init__.py @@ -205,6 +205,7 @@ def _require_successful_response(response): error_data['message'], error_data['requestId'], error_data, + response.status_code, ) elif 500 <= response.status_code < 600: raise HubspotServerError(response.reason, response.status_code) diff --git a/hubspot/connection/exc.py b/hubspot/connection/exc.py index 1bf14c0..fb4cfc1 100644 --- a/hubspot/connection/exc.py +++ b/hubspot/connection/exc.py @@ -31,15 +31,17 @@ class HubspotClientError(HubspotException): :param unicode request_id: :param dict optional error_data: + :param int optional http_status_code: """ - def __init__(self, error_message, request_id, error_data=None): + def __init__(self, error_message, request_id, error_data=None, http_status_code=None): if error_data and 'failureMessages' in error_data: msg = u'%s (%s)' % (error_message, error_data['failureMessages']) else: msg = error_message super(HubspotClientError, self).__init__(msg) + self.http_status_code = http_status_code self.error_message = error_message self.request_id = request_id self.error_data = error_data diff --git a/tests/test_connection.py b/tests/test_connection.py index 489ab61..e8bd790 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -276,6 +276,7 @@ def test_client_error_response(self): eq_(error_message, exception.error_message) eq_(invalid_emails, exception.error_data['invalidEmails']) eq_(failure_messages, exception.error_data['failureMessages']) + eq_(400, exception.http_status_code) def test_str_client_exception_when_no_failure_messages(self): request_id = get_uuid4_str() From 9340ab8f4286bcc1cb154a299119b8d7d69cdc04 Mon Sep 17 00:00:00 2001 From: Francis Devereux Date: Wed, 16 Mar 2016 16:47:54 +0000 Subject: [PATCH 10/16] Add commas to last elements of dicts Makes it easier to add new items. --- tests/test_connection.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index e8bd790..2e586f4 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -250,7 +250,7 @@ def test_client_error_response(self): 'index': 0, 'error': { 'status': 'error', - 'message': 'Email address is invalid' + 'message': 'Email address is invalid', } } ] @@ -261,7 +261,7 @@ def test_client_error_response(self): 'correlationId': '2ebc27ce-cc2d-4b81-99f3-01aa96e05206', 'invalidEmails': invalid_emails, 'failureMessages': failure_messages, - 'requestId': request_id + 'requestId': request_id, } response_data_maker = _ResponseMaker(400, body_deserialization) connection = _MockPortalConnection(response_data_maker) From 80dbe7174f390b8e2549094382a7f47772b0e5a0 Mon Sep 17 00:00:00 2001 From: Francis Devereux Date: Wed, 16 Mar 2016 17:19:49 +0000 Subject: [PATCH 11/16] Add failureMessages to _HUBSPOT_ERROR_RESPONSE_SCHEMA ...so that we validate that it is a list (if it is present). --- hubspot/connection/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hubspot/connection/__init__.py b/hubspot/connection/__init__.py index de369a1..0b33c2d 100644 --- a/hubspot/connection/__init__.py +++ b/hubspot/connection/__init__.py @@ -29,7 +29,7 @@ from requests.adapters import HTTPAdapter from requests.auth import AuthBase from requests.sessions import Session -from voluptuous import Schema +from voluptuous import Optional, Schema from hubspot.connection._validators import Constant from hubspot.connection.exc import HubspotAuthenticationError @@ -48,6 +48,7 @@ 'status': Constant('error'), 'message': unicode, 'requestId': unicode, + Optional('failureMessages'): list, }, required=True, extra=True, From 0d940f5d7df90e13246ae5a9f1b538ffa969ddcb Mon Sep 17 00:00:00 2001 From: Francis Devereux Date: Wed, 16 Mar 2016 17:27:10 +0000 Subject: [PATCH 12/16] Format string using .format() for consistency with existing code --- hubspot/connection/exc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hubspot/connection/exc.py b/hubspot/connection/exc.py index fb4cfc1..bdf57c6 100644 --- a/hubspot/connection/exc.py +++ b/hubspot/connection/exc.py @@ -36,7 +36,7 @@ class HubspotClientError(HubspotException): """ def __init__(self, error_message, request_id, error_data=None, http_status_code=None): if error_data and 'failureMessages' in error_data: - msg = u'%s (%s)' % (error_message, error_data['failureMessages']) + msg = u'{0} ({1!s})'.format(error_message, error_data['failureMessages']) else: msg = error_message super(HubspotClientError, self).__init__(msg) From 4e9e7d11a0ecdfafa076aa30ff10af98de041079 Mon Sep 17 00:00:00 2001 From: Francis Devereux Date: Wed, 16 Mar 2016 17:28:53 +0000 Subject: [PATCH 13/16] Split string to avoid line longer than 80 chars --- tests/test_connection.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index 2e586f4..97f965f 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -271,7 +271,10 @@ def test_client_error_response(self): exception = context_manager.exception eq_(request_id, exception.request_id) - eq_("Errors found processing batch update ([{u'index': 0, u'error': {u'status': u'error', u'message': u'Email address is invalid'}}])", + eq_("Errors found processing batch update " + "([{u'index': 0, u'error': " + "{u'status': u'error'," + " u'message': u'Email address is invalid'}}])", str(exception)) eq_(error_message, exception.error_message) eq_(invalid_emails, exception.error_data['invalidEmails']) From 31d35b68a2edb68ccba96f179fea3cfd15acd30d Mon Sep 17 00:00:00 2001 From: Francis Devereux Date: Wed, 16 Mar 2016 17:29:49 +0000 Subject: [PATCH 14/16] Rename test --- tests/test_connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index 97f965f..a42fce5 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -281,7 +281,7 @@ def test_client_error_response(self): eq_(failure_messages, exception.error_data['failureMessages']) eq_(400, exception.http_status_code) - def test_str_client_exception_when_no_failure_messages(self): + def test_str_client_exception_without_failure_messages(self): request_id = get_uuid4_str() error_message = 'Json node is missing child property' error_data = { From 74c7ce1fb88d421aa7ea3a71f7f4de0477f69f27 Mon Sep 17 00:00:00 2001 From: Francis Devereux Date: Wed, 16 Mar 2016 17:32:41 +0000 Subject: [PATCH 15/16] Update HubspotClientError docstring --- hubspot/connection/exc.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hubspot/connection/exc.py b/hubspot/connection/exc.py index bdf57c6..c7419db 100644 --- a/hubspot/connection/exc.py +++ b/hubspot/connection/exc.py @@ -30,7 +30,8 @@ class HubspotClientError(HubspotException): of 40X, except 401 :param unicode request_id: - :param dict optional error_data: + :param unicode error_message: The error message returned by HubSpot + :param dict optional error_data: The response data returned by HubSpot :param int optional http_status_code: """ From 5e260fd5ecfa209b29257f2852c69dfed9a0c1a1 Mon Sep 17 00:00:00 2001 From: Francis Devereux Date: Thu, 17 Mar 2016 09:45:10 +0000 Subject: [PATCH 16/16] Fix param order in docstring --- hubspot/connection/exc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hubspot/connection/exc.py b/hubspot/connection/exc.py index c7419db..b1afe05 100644 --- a/hubspot/connection/exc.py +++ b/hubspot/connection/exc.py @@ -29,8 +29,8 @@ class HubspotClientError(HubspotException): HubSpot deemed the request invalid. This represents an HTTP response code of 40X, except 401 - :param unicode request_id: :param unicode error_message: The error message returned by HubSpot + :param unicode request_id: :param dict optional error_data: The response data returned by HubSpot :param int optional http_status_code: