diff --git a/ChangeLog b/ChangeLog index 93813b1df..6a15b2667 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ * 0.7.0: - Google Ads v0_7 release. +- Address inconsistent exception handling in Exception and Logging +interceptors. +- Pin `google-api-core` and `grpcio` dependencies. * 0.6.0: - Add configurable logging functionality. diff --git a/google/ads/google_ads/__init__.py b/google/ads/google_ads/__init__.py index 2e059c5d3..0cc0be79f 100644 --- a/google/ads/google_ads/__init__.py +++ b/google/ads/google_ads/__init__.py @@ -19,4 +19,5 @@ import google.ads.google_ads.client import google.ads.google_ads.errors + VERSION = '0.7.0' diff --git a/google/ads/google_ads/client.py b/google/ads/google_ads/client.py index 8fe7e86ae..0646113d1 100644 --- a/google/ads/google_ads/client.py +++ b/google/ads/google_ads/client.py @@ -24,7 +24,7 @@ import google.auth.transport.requests import google.oauth2.credentials import google.ads.google_ads.errors -from google.ads.google_ads.v0.proto.errors import errors_pb2 +from google.ads.google_ads.v0.proto.errors import errors_pb2 as error_protos from google.protobuf.json_format import MessageToJson import grpc @@ -160,7 +160,7 @@ def __init__(self, credentials, developer_token, endpoint=None, developer_token: a str developer token. endpoint: a str specifying an optional alternative API endpoint. login_customer_id: a str specifying a login customer ID. - logging_config: a dict specifying logging config options + logging_config: a dict specifying logging config options. """ _validate_login_customer_id(login_customer_id) @@ -233,7 +233,7 @@ def _get_google_ads_failure(self, trailing_metadata): """Gets the Google Ads failure details if they exist. Args: - trailing_metadata: + trailing_metadata: a tuple of metadatum from the service response. Returns: A GoogleAdsFailure that describes how a GoogleAds API call failed. @@ -241,20 +241,23 @@ def _get_google_ads_failure(self, trailing_metadata): return the failure details, or if the GoogleAdsFailure fails to parse. """ - for kv in trailing_metadata: - if kv[0] == self._FAILURE_KEY: - try: - ga_failure = errors_pb2.GoogleAdsFailure() - ga_failure.ParseFromString(kv[1]) - return ga_failure - except google.protobuf.message.DecodeError: - return None + if trailing_metadata is not None: + for kv in trailing_metadata: + if kv[0] == self._FAILURE_KEY: + try: + ga_failure = error_protos.GoogleAdsFailure() + ga_failure.ParseFromString(kv[1]) + return ga_failure + except google.protobuf.message.DecodeError: + return None + + return None def _get_request_id(self, trailing_metadata): """Gets the request ID for the Google Ads API request. Args: - trailing_metadata: + trailing_metadata: a tuple of metadatum from the service response. Returns: A str request ID associated with the Google Ads API request, or None @@ -266,35 +269,42 @@ def _get_request_id(self, trailing_metadata): return None - def _handle_grpc_exception(self, exception): - """Handles gRPC exceptions of type RpcError by attempting to - convert them to a more readable GoogleAdsException. Certain types of - exceptions are not converted; if the object's trailing metadata does - not indicate that it is a GoogleAdsException, or if it falls under - a certain category of status code, (INTERNAL or RESOURCE_EXHAUSTED). - See documentation for more information about gRPC status codes: - https://github.com/grpc/grpc/blob/master/doc/statuscodes.md + def _handle_grpc_failure(self, response): + """Attempts to convert failed responses to a GoogleAdsException object. + + Handles failed gRPC responses of by attempting to convert them + to a more readable GoogleAdsException. Certain types of exceptions are + not converted; if the object's trailing metadata does not indicate that + it is a GoogleAdsException, or if it falls under a certain category of + status code, (INTERNAL or RESOURCE_EXHAUSTED). See documentation for + more information about gRPC status codes: + https://github.com/grpc/grpc/blob/master/doc/statuscodes.md Args: - exception: an exception of type RpcError. + response: a grpc.Call/grpc.Future instance. Raises: GoogleAdsException: If the exception's trailing metadata indicates that it is a GoogleAdsException. - RpcError: If the exception's trailing metadata is empty or is not - indicative of a GoogleAdsException, or if the exception has a - status code of INTERNAL or RESOURCE_EXHAUSTED. + RpcError: If the exception's is a gRPC exception but the trailing + metadata is empty or is not indicative of a GoogleAdsException, + or if the exception has a status code of INTERNAL or + RESOURCE_EXHAUSTED. + Exception: If not a GoogleAdsException or RpcException the error + will be raised as-is. """ - if exception._state.code not in self._RETRY_STATUS_CODES: - trailing_metadata = exception.trailing_metadata() - google_ads_failure = self._get_google_ads_failure( - trailing_metadata) + status_code = response.code() + exception = response.exception() + + if status_code not in self._RETRY_STATUS_CODES: + trailing_metadata = response.trailing_metadata() + google_ads_failure = self._get_google_ads_failure(trailing_metadata) if google_ads_failure: request_id = self._get_request_id(trailing_metadata) raise google.ads.google_ads.errors.GoogleAdsException( - exception, exception, google_ads_failure, request_id) + exception, response, google_ads_failure, request_id) else: # Raise the original exception if not a GoogleAdsFailure. raise exception @@ -308,6 +318,9 @@ def intercept_unary_unary(self, continuation, client_call_details, request): Overrides abstract method defined in grpc.UnaryUnaryClientInterceptor. + Returns: + A grpc.Call instance representing a service response. + Raises: GoogleAdsException: If the exception's trailing metadata indicates that it is a GoogleAdsException. @@ -315,17 +328,13 @@ def intercept_unary_unary(self, continuation, client_call_details, request): indicative of a GoogleAdsException, or if the exception has a status code of INTERNAL or RESOURCE_EXHAUSTED. """ - try: - response = continuation(client_call_details, request) - except grpc.RpcError as ex: - self._handle_grpc_exception(ex) - - if response.exception(): - # Any exception raised within the continuation function that is not - # an RpcError will be set on the response object and raised here. - raise response.exception() + response = continuation(client_call_details, request) + exception = response.exception() - return response + if exception: + self._handle_grpc_failure(response) + else: + return response class LoggingInterceptor(grpc.UnaryUnaryClientInterceptor): @@ -342,10 +351,10 @@ class LoggingInterceptor(grpc.UnaryUnaryClientInterceptor): 'FaultMessage: %s') def __init__(self, logging_config=None, endpoint=None): - """Initializer for the LoggingInterceptor + """Initializer for the LoggingInterceptor. Args: - logging_config: configuration dictionary for logging + logging_config: configuration dict for logging. endpoint: a str specifying an optional alternative API endpoint. """ self.endpoint = endpoint @@ -353,14 +362,18 @@ def __init__(self, logging_config=None, endpoint=None): logging.config.dictConfig(logging_config) def _get_request_id(self, response, exception): - """Retrieves the request id from a response object + """Retrieves the request id from a response object. + + Returns: + A str of the request_id, or None if there's an exception but the + request_id isn't present. Args: - response: a gRPC response object - exception: a gRPC exception object + response: A grpc.Call/grpc.Future instance. + exception: A grpc.Call instance. """ if exception: - return exception.request_id + return getattr(exception, 'request_id', None) else: trailing_metadata = response.trailing_metadata() for datum in trailing_metadata: @@ -368,111 +381,172 @@ def _get_request_id(self, response, exception): return datum[1] def _get_trailing_metadata(self, response, exception): - """Retrieves trailing metadata from a response object + """Retrieves trailing metadata from a response or exception object. + + If the exception is a GoogleAdsException the trailing metadata will be + on its error object, otherwise it will be on the response object. + + Returns: + A tuple of metadatum representing response header key value pairs. Args: - response: a gRPC response object - exception: a gRPC exception object + response: A grpc.Call/grpc.Future instance. + exception: A grpc.Call instance. """ if exception: - return exception.error.trailing_metadata() + return _get_trailing_metadata_from_interceptor_exception(exception) else: return response.trailing_metadata() + def _get_initial_metadata(self, client_call_details): + """Retrieves the initial metadata from client_call_details. + + Returns an empty tuple if metadata isn't present on the + client_call_details object. + + Returns: + A tuple of metadatum representing request header key value pairs. + + Args: + client_call_details: An instance of grpc.ClientCallDetails. + """ + return getattr(client_call_details, 'metadata', tuple()) + + def _get_call_method(self, client_call_details): + """Retrieves the call method from client_call_details. + + Returns None if the method is not present on the client_call_details + object. + + Returns: + A str with the call method or None if it isn't present. + + Args: + client_call_details: An instance of grpc.ClientCallDetails. + """ + return getattr(client_call_details, 'method', None) + + def _get_customer_id(self, request): + """Retrieves the customer_id from the grpc request. + + Returns None if a customer_id is not present on the request object. + + Returns: + A str with the customer id from the request or None if it isn't + present. + + Args: + request: An instance of a request proto message. + """ + return getattr(request, 'customer_id', None) + def _parse_response_to_json(self, response, exception): - """Parses response object to JSON + """Parses response object to JSON. + + Returns: + A str of JSON representing a response or exception from the + service. Args: - response: a gRPC response object - exception: a gRPC exception object + response: A grpc.Call/grpc.Future instance. + exception: A grpc.Call instance. """ if exception: - return _parse_message_to_json(exception.failure) + # try to retrieve the .failure property of a GoogleAdsFailure. + failure = getattr(exception, 'failure', None) + + if failure: + return _parse_message_to_json(failure) + else: + # if exception.failure isn't present then it's likely this is a + # transport error with a .debug_error_string method. + try: + debug_string = exception.debug_error_string() + return _parse_to_json(json.loads(debug_string)) + except (AttributeError, ValueError): + # if both attempts to retrieve serializable error data fail + # then simply return an empty JSON string + return '{}' else: return _parse_message_to_json(response.result()) + def _get_fault_message(self, exception): + """Retrieves a fault/error message from an exception object. + + Returns None if no error message can be found on the exception. + + Returns: + A str with an error message or None if one cannot be found. + + Args: + response: A grpc.Call/grpc.Future instance. + exception: A grpc.Call instance. + """ + try: + return exception.failure.errors[0].message + except AttributeError: + try: + return exception.details() + except AttributeError: + return None + def _log_successful_request(self, method, customer_id, metadata_json, request_id, request_json, trailing_metadata_json, response_json): - """Handles logging of a successful request + """Handles logging of a successful request. Args: - method: the gRPC method of the request - customer_id: the customer ID associated with the request - metadata_json: request metadata (i.e. headers) in JSON form - request_id: unique ID for the request provided in the response - request_json: the request object in JSON form - trailing_metadata_json: metadata from the response as JSON - response_json: the response object as JSON + method: The method of the request. + customer_id: The customer ID associated with the request. + metadata_json: A JSON str of initial_metadata. + request_id: A unique ID for the request provided in the response. + request_json: A JSON str of the request message. + trailing_metadata_json: A JSON str of trailing_metadata. + response_json: A JSON str of the the response message. """ - _logger.debug(self._FULL_REQUEST_LOG_LINE - % ( - method, - self.endpoint, - metadata_json, - request_json, - trailing_metadata_json, - response_json - )) - - _logger.info(self._SUMMARY_LOG_LINE - % ( - customer_id, - self.endpoint, - method, - request_id, - False, - None - )) + _logger.debug(self._FULL_REQUEST_LOG_LINE % (method, self.endpoint, + metadata_json, request_json, trailing_metadata_json, + response_json)) + + _logger.info(self._SUMMARY_LOG_LINE % (customer_id, self.endpoint, + method, request_id, False, None)) def _log_failed_request(self, method, customer_id, metadata_json, request_id, request_json, trailing_metadata_json, response_json, fault_message): - """Handles logging of a failed request + """Handles logging of a failed request. Args: - method: the gRPC method of the request - customer_id: the customer ID associated with the request - metadata_json: request metadata (i.e. headers) in JSON form - request_id: unique ID for the request provided in the response - request_json: the request object in JSON form - trailing_metadata_json: metadata from the response as JSON - response_json: the response object as JSON - fault_message: the error message from the failed request + method: The method of the request. + customer_id: The customer ID associated with the request. + metadata_json: A JSON str of initial_metadata. + request_id: A unique ID for the request provided in the response. + request_json: A JSON str of the request message. + trailing_metadata_json: A JSON str of trailing_metadata. + response_json: A JSON str of the the response message. + fault_message: A str error message from a failed request. """ - _logger.warning(self._SUMMARY_LOG_LINE - % ( - customer_id, - self.endpoint, - method, - request_id, - True, - fault_message - )) - - _logger.info(self._FULL_FAULT_LOG_LINE - % ( - method, - self.endpoint, - metadata_json, - request_json, - trailing_metadata_json, - response_json - )) + _logger.info(self._FULL_FAULT_LOG_LINE % (method, self.endpoint, + metadata_json, request_json, trailing_metadata_json, + response_json)) + + _logger.warning(self._SUMMARY_LOG_LINE % (customer_id, self.endpoint, + method, request_id, True, fault_message)) def _log_request(self, client_call_details, request, response, exception): - """Handles logging all requests + """Handles logging all requests. Args: - client_call_details: information about the client call - request: an instance of a gRPC request - response: a gRPC response object - exception: a gRPC exception object + client_call_details: An instance of grpc.ClientCallDetails. + request: An instance of a request proto message. + response: A grpc.Call/grpc.Future instance. + exception: A grpc.Call instance. """ - method = client_call_details.method - customer_id = getattr(request, 'customer_id', None) - metadata_json = _parse_metadata_to_json(client_call_details.metadata) + method = self._get_call_method(client_call_details) + customer_id = self._get_customer_id(request) + initial_metadata = self._get_initial_metadata(client_call_details) + initial_metadata_json = _parse_metadata_to_json(initial_metadata) request_json = _parse_message_to_json(request) request_id = self._get_request_id(response, exception) response_json = self._parse_response_to_json(response, exception) @@ -480,30 +554,24 @@ def _log_request(self, client_call_details, request, response, exception): trailing_metadata_json = _parse_metadata_to_json(trailing_metadata) if exception: - fault_message = exception.failure.errors[0].message - self._log_failed_request( - method, - customer_id, - metadata_json, - request_id, - request_json, - trailing_metadata_json, - response_json, - fault_message) + fault_message = self._get_fault_message(exception) + self._log_failed_request(method, customer_id, initial_metadata_json, + request_id, request_json, + trailing_metadata_json, response_json, + fault_message) else: - self._log_successful_request( - method, - customer_id, - metadata_json, - request_id, - request_json, - trailing_metadata_json, - response_json) + self._log_successful_request(method, customer_id, + initial_metadata_json, request_id, + request_json, trailing_metadata_json, + response_json) def intercept_unary_unary(self, continuation, client_call_details, request): """Intercepts and logs API interactions. Overrides abstract method defined in grpc.UnaryUnaryClientInterceptor. + + Returns: + A grpc.Call/grpc.Future instance representing a service response. """ response = continuation(client_call_details, request) if _logger.isEnabledFor(logging.WARNING): @@ -536,6 +604,9 @@ def intercept_unary_unary(self, continuation, client_call_details, request): """Intercepts and appends custom metadata. Overrides abstract method defined in grpc.UnaryUnaryClientInterceptor. + + Returns: + A grpc.Call/grpc.Future instance representing a service response. """ if client_call_details.metadata is None: metadata = [] @@ -559,10 +630,30 @@ class _ClientCallDetails( '_ClientCallDetails', ('method', 'timeout', 'metadata', 'credentials')), grpc.ClientCallDetails): - """An wrapper class for initializing a new ClientCallDetails instance.""" + """A wrapper class for initializing a new ClientCallDetails instance.""" pass +def _get_trailing_metadata_from_interceptor_exception(exception): + """Retrieves trailing metadata from an exception object. + + Args: + exception: an instance of grpc.Call. + + Returns: + A tuple of trailing metadata key value pairs. + """ + try: + return exception.error.trailing_metadata() + except AttributeError: + try: + return exception.trailing_metadata() + except AttributeError: + # if trailing metadata is not found in either location then + # return an empty tuple + return tuple() + + def _get_version(name): """Returns the given API version. @@ -587,8 +678,8 @@ def _validate_login_customer_id(login_customer_id): login_customer_id: a str from config indicating a login customer ID. Raises: - ValueError: If the login customer ID is not - an int in the range 0 - 9999999999. + ValueError: If the login customer ID is not an int in the + range 0 - 9999999999. """ if login_customer_id is not None: if not login_customer_id.isdigit() or len(login_customer_id) != 10: @@ -597,13 +688,32 @@ def _validate_login_customer_id(login_customer_id): 'as a string, i.e. "1234567890"') +def _parse_to_json(obj): + """Parses a serializable object into a consistently formatted JSON string. + + Returns: + A str of formatted JSON serialized from the given object. + + Args: + obj: an object or dict. + """ + def default_serializer(value): + if isinstance(value, bytes): + return value.decode(errors='ignore') + else: + return None + + return str(json.dumps(obj, indent=2, sort_keys=True, ensure_ascii=False, + default=default_serializer, separators=(',', ': '))) + + def _parse_metadata_to_json(metadata): - """Parses metadata from a gRPC requests and responses to a JSON string. - Obscures the value for "developer-token". + """Parses metadata from gRPC request and response messages to a JSON str. + + Obscures the value for "developer-token". Args: - metadata: a list of tuples of metadata information from a - gRPC response + metadata: a tuple of metadatum. """ SENSITIVE_INFO_MASK = 'REDACTED' metadata_dict = {} @@ -619,18 +729,16 @@ def _parse_metadata_to_json(metadata): value = datum[1] metadata_dict[key] = value - return json.dumps( - metadata_dict, indent=2, sort_keys=True, ensure_ascii=False, - separators=(',', ': ')) + return _parse_to_json(metadata_dict) def _parse_message_to_json(message): """Parses a gRPC request object to a JSON string. Args: - request: an instance of the SearchGoogleAdsRequest type + request: an instance of a request proto message, for example + a SearchGoogleAdsRequest or a MutateAdGroupAdsRequest. """ - json = MessageToJson(message) + json = MessageToJson(message) json = json.replace(', \n', ',\n') return json - diff --git a/setup.py b/setup.py index 8fb2a4ae4..70adff7a1 100644 --- a/setup.py +++ b/setup.py @@ -18,9 +18,10 @@ install_requires = [ 'enum34; python_version < "3.4"', - 'google-auth-oauthlib>=0.0.1,<1.0.0', - 'google-api-core[grpc] >= 1.4.0, < 2.0.0dev', - 'PyYAML >=4.2b1, < 5.0', + 'google-auth-oauthlib >= 0.0.1, < 1.0.0', + 'google-api-core == 1.7.0', + 'grpcio == 1.18.0', + 'PyYAML >= 4.2b1, < 5.0', ] tests_require = [ diff --git a/tests/client_test.py b/tests/client_test.py index c1d0d0eee..baf9a8988 100644 --- a/tests/client_test.py +++ b/tests/client_test.py @@ -19,15 +19,20 @@ import yaml import json import logging +from unittest import TestCase + +import grpc +from pyfakefs.fake_filesystem_unittest import TestCase as FileTestCase import google.ads.google_ads.client import google.ads.google_ads.v0 from google.ads.google_ads.v0.proto.services import google_ads_service_pb2 -from unittest import TestCase -from pyfakefs.fake_filesystem_unittest import TestCase as FileTestCase +from google.ads.google_ads.v0.proto.errors import errors_pb2 as error_protos +from google.ads.google_ads.errors import GoogleAdsException class ModuleLevelTest(TestCase): + def test_parse_metadata_to_json(self): mock_metadata = [ ('x-goog-api-client', @@ -421,6 +426,7 @@ def test_intercept_unary_unary(self): class LoggingInterceptorTest(TestCase): """Tests for the google.ads.googleads.client.LoggingInterceptor class.""" + _MOCK_CONFIG = {'test': True} _MOCK_ENDPOINT = 'www.test-endpoint.com' _MOCK_INITIAL_METADATA = [('developer-token', '123456'), @@ -430,31 +436,67 @@ class LoggingInterceptorTest(TestCase): _MOCK_METHOD = 'test/method' _MOCK_TRAILING_METADATA = (('request-id', _MOCK_REQUEST_ID),) _MOCK_ERROR_MESSAGE = 'Test error message' + _MOCK_TRANSPORT_ERROR_MESSAGE = u'Received RST_STREAM with error code 2' + _MOCK_DEBUG_ERROR_STRING = u'{"description":"Error received from peer"}' def _create_test_interceptor(self, config=_MOCK_CONFIG, endpoint=_MOCK_ENDPOINT): + """Creates a LoggingInterceptor instance. + + Accepts parameters that are used to override defaults when needed + for testing. + + Returns: + A LoggingInterceptor instance. + + Args: + config: A dict configuration + endpoint: A str representing an endpoint + """ return google.ads.google_ads.client.LoggingInterceptor(config, endpoint) def _get_mock_client_call_details(self): + """Generates a mock client_call_details object for use in tests. + + Returns: + A Mock instance with "method" and "metadata" attributes. + """ mock_client_call_details = mock.Mock() mock_client_call_details.method = self._MOCK_METHOD mock_client_call_details.metadata = self._MOCK_INITIAL_METADATA return mock_client_call_details def _get_mock_request(self): + """Generates a mock request object for use in tests. + + Returns: + A Mock instance with a "customer_id" attribute. + """ mock_request = mock.Mock() mock_request.customer_id = self._MOCK_CUSTOMER_ID return mock_request - def _get_trailing_metadata_fn(self, failed=False): + def _get_trailing_metadata_fn(self): + """Generates a mock trailing_metadata function used for testing. + + Returns: + A function that returns a tuple of mock metadata. + """ def mock_trailing_metadata_fn(): - if failed: - return None - mock_trailing_metadata = self._MOCK_TRAILING_METADATA - return mock_trailing_metadata + return self._MOCK_TRAILING_METADATA + return mock_trailing_metadata_fn def _get_mock_exception(self): + """Generates a mock GoogleAdsException exception instance for testing. + + Returns: + A Mock instance with the following attributes - "message", + "request_id", "failure", and "error." The "failure" attribute has an + "error" attribute that is an array of mock error objects, and the + "error" attribute is an object with a "trailing_metadata" method + that returns a tuble of mock metadata. + """ exception = mock.Mock() error = mock.Mock() error.message = self._MOCK_ERROR_MESSAGE @@ -462,11 +504,53 @@ def _get_mock_exception(self): exception.failure = mock.Mock() exception.failure.errors = [error] exception.error = mock.Mock() - exception.error.trailing_metadata = self._get_trailing_metadata_fn( - failed=True) + exception.error.trailing_metadata = self._get_trailing_metadata_fn() + return exception + + def _get_mock_transport_exception(self): + """Generates a mock gRPC transport error. + + Specifically an error not generated by the Google Ads API and that + is not an instance of GoogleAdsException. + + Returns: + A Mock instance with mock "debug_error_string," "details," and + trailing_metadata" methods. + """ + def _mock_debug_error_string(): + return self._MOCK_DEBUG_ERROR_STRING + + def _mock_details(): + return self._MOCK_TRANSPORT_ERROR_MESSAGE + + def _mock_trailing_metadata(): + return self._MOCK_TRAILING_METADATA + + exception = mock.Mock() + exception.debug_error_string = _mock_debug_error_string + exception.details = _mock_details + exception.trailing_metadata = _mock_trailing_metadata + # These attributes are explicitly deleted because they will otherwise + # get mocked automatically and not generate AttributeErrors that trigger + # default values in certain helper methods. + del exception.error + del exception.failure return exception def _get_mock_response(self, failed=False): + """Generates a mock response object for use in tests. + + Accepts a "failed" param that tells the returned mocked response to + mimic a failed response. + + Returns: + A Mock instance with mock "exception" and "trailing_metadata" + methods + + Args: + failed: a bool indicating whether the mock response should be in a + failed state or not. Default is False. + """ def mock_exception_fn(): if failed: return self._get_mock_exception() @@ -474,10 +558,22 @@ def mock_exception_fn(): mock_response = mock.Mock() mock_response.exception = mock_exception_fn - mock_response.trailing_metadata = self._get_trailing_metadata_fn(failed) + mock_response.trailing_metadata = self._get_trailing_metadata_fn() return mock_response def _get_mock_continuation_fn(self, fail=False): + """Generates a mock continuation function for use in tests. + + Accepts a "failed" param that tell the function to return a failed + mock response or not. + + Returns: + A function that returns a mock response object. + + Args: + failed: a bool indicating whether the function should return a + response that mocks a failure. + """ def mock_continuation_fn(client_call_details, request): mock_response = self._get_mock_response(fail) return mock_response @@ -485,11 +581,15 @@ def mock_continuation_fn(client_call_details, request): return mock_continuation_fn def test_init_no_config(self): + """Unconfigured LoggingInterceptor should not call logging.dictConfig. + """ with mock.patch('logging.config.dictConfig') as mock_dictConfig: interceptor = google.ads.google_ads.client.LoggingInterceptor() mock_dictConfig.assert_not_called() def test_init_with_config(self): + """Configured LoggingInterceptor should call logging.dictConfig. + """ config = {'test': True} with mock.patch('logging.config.dictConfig') as mock_dictConfig: interceptor = google.ads.google_ads.client.LoggingInterceptor( @@ -497,6 +597,11 @@ def test_init_with_config(self): mock_dictConfig.assert_called_once_with(config) def test_intercept_unary_unary_unconfigured(self): + """No _logger methods should be called. + + When intercepting requests, no logging methods should be called if + LoggingInterceptor was initialized without a configuration. + """ mock_client_call_details = self._get_mock_client_call_details() mock_continuation_fn = self._get_mock_continuation_fn() mock_request = self._get_mock_request() @@ -517,8 +622,12 @@ def test_intercept_unary_unary_unconfigured(self): logger_spy.info.assert_not_called() logger_spy.warning.assert_not_called() - def test_intercept_unary_unary_successful_request(self): + """_logger.info and _logger.debug should be called. + + LoggingInterceptor should call _logger.info and _logger.debug with + a specific str parameter when a request succeeds. + """ mock_client_call_details = self._get_mock_client_call_details() mock_continuation_fn = self._get_mock_continuation_fn() mock_request = self._get_mock_request() @@ -540,31 +649,28 @@ def test_intercept_unary_unary_successful_request(self): mock_logger.info.assert_called_once_with( interceptor._SUMMARY_LOG_LINE - % ( - self._MOCK_CUSTOMER_ID, - self._MOCK_ENDPOINT, - mock_client_call_details.method, - self._MOCK_REQUEST_ID, - False, - None - ) - ) + % (self._MOCK_CUSTOMER_ID, self._MOCK_ENDPOINT, + mock_client_call_details.method, self._MOCK_REQUEST_ID, + False, None)) + + initial_metadata = (google.ads.google_ads.client. + _parse_metadata_to_json( + mock_client_call_details.metadata)) + trailing_metadata = (google.ads.google_ads.client. + _parse_metadata_to_json( + mock_trailing_metadata)) mock_logger.debug.assert_called_once_with( interceptor._FULL_REQUEST_LOG_LINE - % ( - self._MOCK_METHOD, - self._MOCK_ENDPOINT, - google.ads.google_ads.client. - _parse_metadata_to_json(mock_client_call_details.metadata), - mock_json_message, - google.ads.google_ads.client. - _parse_metadata_to_json(mock_trailing_metadata), - mock_json_message - ) - ) + % (self._MOCK_METHOD, self._MOCK_ENDPOINT, initial_metadata, + mock_json_message, trailing_metadata, mock_json_message)) def test_intercept_unary_unary_failed_request(self): + """_logger.warning and _logger.info should be called. + + LoggingInterceptor should call _logger.warning and _logger.info with + a specific str parameter when a request fails. + """ mock_client_call_details = self._get_mock_client_call_details() mock_continuation_fn = self._get_mock_continuation_fn(fail=True) mock_request = self._get_mock_request() @@ -586,27 +692,357 @@ def test_intercept_unary_unary_failed_request(self): mock_logger.warning.assert_called_once_with( interceptor._SUMMARY_LOG_LINE - % ( - self._MOCK_CUSTOMER_ID, - self._MOCK_ENDPOINT, - mock_client_call_details.method, - self._MOCK_REQUEST_ID, - True, - self._MOCK_ERROR_MESSAGE - ) - ) + % (self._MOCK_CUSTOMER_ID, self._MOCK_ENDPOINT, + mock_client_call_details.method, self._MOCK_REQUEST_ID, + True, self._MOCK_ERROR_MESSAGE)) + + initial_metadata = (google.ads.google_ads.client. + _parse_metadata_to_json( + mock_client_call_details.metadata)) + trailing_metadata = (google.ads.google_ads.client. + _parse_metadata_to_json( + mock_trailing_metadata)) mock_logger.info.assert_called_once_with( interceptor._FULL_FAULT_LOG_LINE - % ( - self._MOCK_METHOD, - self._MOCK_ENDPOINT, - google.ads.google_ads.client. - _parse_metadata_to_json(mock_client_call_details.metadata), - mock_json_message, - google.ads.google_ads.client. - _parse_metadata_to_json(mock_trailing_metadata), - mock_json_message - ) - ) + % (self._MOCK_METHOD, self._MOCK_ENDPOINT, initial_metadata, + mock_json_message, trailing_metadata, mock_json_message)) + + def test_get_initial_metadata(self): + """_Returns a tuple of metadata from client_call_details.""" + with mock.patch('logging.config.dictConfig'): + mock_client_call_details = mock.Mock() + mock_client_call_details.metadata = self._MOCK_INITIAL_METADATA + interceptor = self._create_test_interceptor() + result = interceptor._get_initial_metadata(mock_client_call_details) + self.assertEqual(result, self._MOCK_INITIAL_METADATA) + + def test_get_initial_metadata_none(self): + """Returns an empty tuple if initial_metadata isn't present.""" + with mock.patch('logging.config.dictConfig'): + mock_client_call_details = {} + interceptor = self._create_test_interceptor() + result = interceptor._get_initial_metadata(mock_client_call_details) + self.assertEqual(result, tuple()) + + def test_get_call_method(self): + """Returns a str of the call method from client_call_details""" + with mock.patch('logging.config.dictConfig'): + mock_client_call_details = mock.Mock() + mock_client_call_details.method = self._MOCK_METHOD + interceptor = self._create_test_interceptor() + result = interceptor._get_call_method(mock_client_call_details) + self.assertEqual(result, self._MOCK_METHOD) + + def test_get_call_method_none(self): + """Returns None if method is not present on client_call_details.""" + with mock.patch('logging.config.dictConfig'): + mock_client_call_details = {} + interceptor = self._create_test_interceptor() + result = interceptor._get_call_method(mock_client_call_details) + self.assertEqual(result, None) + + def test_get_request_id(self): + """Returns a request ID str from a response object.""" + with mock.patch('logging.config.dictConfig'): + mock_response = self._get_mock_response() + mock_exception = None + interceptor = self._create_test_interceptor() + result = interceptor._get_request_id(mock_response, mock_exception) + self.assertEqual(result, self._MOCK_REQUEST_ID) + + def test_get_request_id_google_ads_failure(self): + """Returns a request ID str from a GoogleAdsException instance.""" + with mock.patch('logging.config.dictConfig'): + mock_response = self._get_mock_response(failed=True) + mock_exception = mock_response.exception() + interceptor = self._create_test_interceptor() + result = interceptor._get_request_id(mock_response, mock_exception) + self.assertEqual(result, self._MOCK_REQUEST_ID) + + def test_get_request_id_transport_failure(self): + """Returns None if there is no request_id on the exception.""" + with mock.patch('logging.config.dictConfig'): + mock_response = self._get_mock_response(failed=True) + mock_exception = mock_response.exception() + # exceptions on transport errors have no request_id because they + # don't interact with a server that can provide one. + del mock_exception.request_id + interceptor = self._create_test_interceptor() + result = interceptor._get_request_id(mock_response, mock_exception) + self.assertEqual(result, None) + + def test_parse_response_to_json(self): + """Calls MessageToJson with a successful response message.""" + with mock.patch('logging.config.dictConfig'), \ + mock.patch( + 'google.ads.google_ads.client.MessageToJson') as mock_formatter: + mock_response = self._get_mock_response() + mock_exception = mock_response.exception() + interceptor = self._create_test_interceptor() + interceptor._parse_response_to_json(mock_response, mock_exception) + mock_formatter.assert_called_once_with(mock_response.result()) + + def test_parse_response_to_json_google_ads_failure(self): + """Calls MessageToJson with a GoogleAdsException.""" + with mock.patch('logging.config.dictConfig'), \ + mock.patch( + 'google.ads.google_ads.client.MessageToJson') as mock_formatter: + mock_response = mock.Mock() + mock_exception = self._get_mock_exception() + interceptor = self._create_test_interceptor() + interceptor._parse_response_to_json(mock_response, mock_exception) + mock_formatter.assert_called_once_with(mock_exception.failure) + + def test_parse_response_to_json_transport_failure(self): + """ Calls _parse_to_json with transport error's debug_error_string.""" + with mock.patch('logging.config.dictConfig'), \ + mock.patch( + 'google.ads.google_ads.client._parse_to_json') as mock_parser: + mock_response = mock.Mock() + mock_exception = self._get_mock_transport_exception() + interceptor = self._create_test_interceptor() + interceptor._parse_response_to_json(mock_response, mock_exception) + mock_parser.assert_called_once_with( + json.loads(self._MOCK_DEBUG_ERROR_STRING)) + + def test_parse_response_to_json_unknown_failure(self): + """Returns an empty JSON string if nothing can be parsed to JSON.""" + with mock.patch('logging.config.dictConfig'): + mock_response = mock.Mock() + mock_exception = mock.Mock() + del mock_exception.failure + del mock_exception.debug_error_string + interceptor = self._create_test_interceptor() + result = interceptor._parse_response_to_json( + mock_response, mock_exception) + self.assertEqual(result, '{}') + + def test_get_trailing_metadata(self): + """Retrieves metadata from a response object.""" + with mock.patch('logging.config.dictConfig'): + mock_response = self._get_mock_response() + mock_exception = mock_response.exception() + interceptor = self._create_test_interceptor() + result = interceptor._get_trailing_metadata( + mock_response, mock_exception) + self.assertEqual(result, self._MOCK_TRAILING_METADATA) + + def test_get_trailing_metadata_google_ads_failure(self): + """Retrieves metadata from a failed response.""" + with mock.patch('logging.config.dictConfig'): + mock_response = self._get_mock_response(failed=True) + mock_exception = mock_response.exception() + interceptor = self._create_test_interceptor() + result = interceptor._get_trailing_metadata( + mock_response, mock_exception) + self.assertEqual(result, self._MOCK_TRAILING_METADATA) + + def test_get_trailing_metadata_transport_failure(self): + """Retrieves metadata from a transport error.""" + with mock.patch('logging.config.dictConfig'): + mock_response = mock.Mock() + mock_exception = self._get_mock_transport_exception() + interceptor = self._create_test_interceptor() + result = interceptor._get_trailing_metadata( + mock_response, mock_exception) + self.assertEqual(result, self._MOCK_TRAILING_METADATA) + + def test_get_trailing_metadata_unknown_failure(self): + """Returns an empty tuple if metadata cannot be found.""" + with mock.patch('logging.config.dictConfig'): + mock_response = {} + mock_exception = self._get_mock_transport_exception() + del mock_exception.trailing_metadata + interceptor = self._create_test_interceptor() + result = interceptor._get_trailing_metadata( + mock_response, mock_exception) + self.assertEqual(result, tuple()) + + def test_get_fault_message(self): + """Returns None if an error message cannot be found.""" + with mock.patch('logging.config.dictConfig'): + mock_exception = None + interceptor = self._create_test_interceptor() + result = interceptor._get_fault_message(mock_exception) + self.assertEqual(result, None) + + def test_get_fault_message_google_ads_failure(self): + """Retrieves an error message from a GoogleAdsException.""" + with mock.patch('logging.config.dictConfig'): + mock_exception = self._get_mock_exception() + interceptor = self._create_test_interceptor() + result = interceptor._get_fault_message(mock_exception) + self.assertEqual(result, self._MOCK_ERROR_MESSAGE) + + def test_get_fault_message_transport_failure(self): + """Retrieves an error message from a transport error object.""" + with mock.patch('logging.config.dictConfig'): + mock_exception = self._get_mock_transport_exception() + interceptor = self._create_test_interceptor() + result = interceptor._get_fault_message(mock_exception) + self.assertEqual(result, self._MOCK_TRANSPORT_ERROR_MESSAGE) + + +class ExceptionInterceptorTest(TestCase): + """Tests for the google.ads.googleads.client.ExceptionInterceptor class.""" + + _MOCK_FAILURE_VALUE = b"\n \n\x02\x08\x10\x12\x1aInvalid customer ID '123'." + + def _create_test_interceptor(self): + """Creates and returns an ExceptionInterceptor instance + + Returns: + An ExceptionInterceptor instance. + """ + return google.ads.google_ads.client.ExceptionInterceptor() + + def test_init_(self): + """Tests that the interceptor initializes properly""" + interceptor = self._create_test_interceptor() + self.assertEqual(interceptor._RETRY_STATUS_CODES, + (grpc.StatusCode.INTERNAL, + grpc.StatusCode.RESOURCE_EXHAUSTED)) + + def test_get_request_id(self): + """_get_request_id obtains a request ID from a metadata tuple""" + mock_metadata = (('request-id', '123456'),) + interceptor = self._create_test_interceptor() + result = interceptor._get_request_id(mock_metadata) + self.assertEqual(result, '123456') + + def test_get_request_id_no_id(self): + """Returns None if the given metadata does not contain a request ID.""" + mock_metadata = (('another-key', 'another-val'),) + interceptor = self._create_test_interceptor() + result = interceptor._get_request_id(mock_metadata) + self.assertEqual(result, None) + + def test_get_google_ads_failure(self): + """Obtains the content of a google ads failure from metadata.""" + interceptor = self._create_test_interceptor() + mock_metadata = ((interceptor._FAILURE_KEY, self._MOCK_FAILURE_VALUE),) + result = interceptor._get_google_ads_failure(mock_metadata) + self.assertIsInstance(result, error_protos.GoogleAdsFailure) + + def test_get_google_ads_failure_decode_error(self): + """Returns none if the google ads failure cannot be decoded.""" + interceptor = self._create_test_interceptor() + mock_failure_value = self._MOCK_FAILURE_VALUE + b'1234' + mock_metadata = ((interceptor._FAILURE_KEY, mock_failure_value),) + result = interceptor._get_google_ads_failure(mock_metadata) + self.assertEqual(result, None) + + def test_get_google_ads_failure_no_failure_key(self): + """Returns None if an error cannot be found in metadata.""" + mock_metadata = (('another-key', 'another-val'),) + interceptor = self._create_test_interceptor() + result = interceptor._get_google_ads_failure(mock_metadata) + self.assertEqual(result, None) + + def test_get_google_ads_failure_with_None(self): + """Returns None if None is passed.""" + interceptor = self._create_test_interceptor() + result = interceptor._get_google_ads_failure(None) + self.assertEqual(result, None) + + def test_handle_grpc_failure(self): + """Raises non-retryable GoogleAdsFailures as GoogleAdsExceptions.""" + mock_error_message = self._MOCK_FAILURE_VALUE + + class MockRpcErrorResponse(grpc.RpcError): + def code(self): + return grpc.StatusCode.INVALID_ARGUMENT + + def trailing_metadata(self): + return ((interceptor._FAILURE_KEY, mock_error_message),) + + def exception(self): + return self + + interceptor = self._create_test_interceptor() + + self.assertRaises(GoogleAdsException, + interceptor._handle_grpc_failure, + MockRpcErrorResponse()) + + def test_handle_grpc_failure_retryable(self): + """Raises retryable exceptions as-is.""" + class MockRpcErrorResponse(grpc.RpcError): + def code(self): + return grpc.StatusCode.INTERNAL + + def exception(self): + return self + + interceptor = self._create_test_interceptor() + + self.assertRaises(MockRpcErrorResponse, + interceptor._handle_grpc_failure, + MockRpcErrorResponse()) + + def test_handle_grpc_failure_not_google_ads_failure(self): + """Raises as-is non-retryable non-GoogleAdsFailure exceptions.""" + class MockRpcErrorResponse(grpc.RpcError): + def code(self): + return grpc.StatusCode.INVALID_ARGUMENT + + def trailing_metadata(self): + return (('bad-failure-key', 'arbitrary-value'),) + + def exception(self): + return self + + interceptor = self._create_test_interceptor() + + self.assertRaises(MockRpcErrorResponse, + interceptor._handle_grpc_failure, + MockRpcErrorResponse()) + + def test_intercept_unary_unary_response_is_exception(self): + """If response.exception() is not None exception is handled.""" + mock_exception = grpc.RpcError() + + class MockResponse(): + def exception(self): + return mock_exception + + mock_request = mock.Mock() + mock_client_call_details = mock.Mock() + mock_response = MockResponse() + + def mock_continuation(client_call_details, request): + del client_call_details + del request + return mock_response + + interceptor = self._create_test_interceptor() + + with mock.patch.object(interceptor, '_handle_grpc_failure'): + interceptor.intercept_unary_unary( + mock_continuation, mock_client_call_details, mock_request) + + interceptor._handle_grpc_failure.assert_called_once_with( + mock_response) + + def test_intercept_unary_unary_response_is_successful(self): + """If response.exception() is None response is returned.""" + class MockResponse(): + def exception(self): + return None + + mock_request = mock.Mock() + mock_client_call_details = mock.Mock() + mock_response = MockResponse() + + def mock_continuation(client_call_details, request): + del client_call_details + del request + return mock_response + + interceptor = self._create_test_interceptor() + + result = interceptor.intercept_unary_unary( + mock_continuation, mock_client_call_details, mock_request) + self.assertEqual(result, mock_response)