Fix Response Error Caching in Streaming Interceptors (#248)

Author: BenRKarl

google/ads/google_ads/interceptors/interceptor.py

@@ -155,7 +155,6 @@ def __init__(self, api_version):
         self._error_protos = None
         self._failure_key = (
             f'google.ads.googleads.{api_version}.errors.googleadsfailure-bin')
-        self._exception = None
         self._api_version = api_version
 
     def _get_error_from_response(self, response):
@@ -182,9 +181,6 @@ def _get_error_from_response(self, response):
             Exception: If not a GoogleAdsException or RpcException the error
                 will be raised as-is.
         """
-        if self._exception:
-            return self._exception
-
         status_code = response.code()
         response_exception = response.exception()
 
@@ -200,20 +196,19 @@ def _get_error_from_response(self, response):
                 # library-specific Error type for easy handling. These errors
                 # originate from the Google Ads API and are often caused by
                 # invalid requests.
-                self._exception = GoogleAdsException(
+                return GoogleAdsException(
                     response_exception, response, google_ads_failure,
                     request_id)
             else:
                 # Raise the original exception if not a GoogleAdsFailure. This
                 # type of error is generally caused by problems at the request
                 # level, such as when an invalid endpoint is given.
-                self._exception = response_exception
+                return response_exception
         else:
             # Raise the original exception if error has status code
             # INTERNAL or RESOURCE_EXHAUSTED, meaning that
-            self._exception = response_exception
+            return response_exception
 
-        return self._exception
 
     def _get_google_ads_failure(self, trailing_metadata):
         """Gets the Google Ads failure details if they exist.

tests/interceptors/interceptor_test.py

@@ -18,6 +18,8 @@
 import mock
 from unittest import TestCase
 
+import grpc
+
 from google.ads.google_ads.client import _DEFAULT_VERSION as default_version
 from google.ads.google_ads.interceptors.interceptor import Interceptor
 
@@ -105,3 +107,25 @@ def test_deferred_error_proto_module_load(self):
             import_mock.assert_called_once_with('google.ads.google_ads.'
                                                 f'{default_version}.proto.'
                                                 'errors.errors_pb2')
+
+    def test_get_error_from_response_does_not_cache_error(self):
+        """Ensures errors are not cached across requests.."""
+        interceptor = Interceptor(default_version)
+
+        class MockRpcErrorResponse(grpc.RpcError):
+            def code(self):
+                return grpc.StatusCode.INVALID_ARGUMENT
+
+            def trailing_metadata(self):
+                return ((
+                    interceptor._failure_key,
+                    b"\n \n\x02\x08\x10\x12\x1aInvalid customer ID '123'."),)
+
+            def exception(self):
+                return self
+
+        response = MockRpcErrorResponse()
+
+        first_result = interceptor._get_error_from_response(response)
+        second_result = interceptor._get_error_from_response(response)
+        self.assertIsNot(first_result, second_result)