Merge pull request #16 from deftinc/test-rate-limiting

Author: vinbarnes

README.md

@@ -67,7 +67,7 @@ except FeatureNotFound as err:
   # Do what we want to do when the feature doesn't exist
 ```
 
-### PostApiClientError
+### PosthogApiClientError
 For the `PosthogAdapter` in particular it will raise error if it was unable to reach the Posthog API. These get bubbled up as `PosthogAPIClientError`.
 
 ```python
@@ -79,6 +79,18 @@ except PosthogAPIClientError as err:
   # Handle the error -- define default behavior in outage
 ```
 
+### RateLimitError
+Again, specific to the `PosthogAdapter` it will raise an error if the account is rate limited by the Posthog API. These get bubbled up as `RateLimitError`.
+
+```python
+from feature_gate.clients.posthog_api_client import RateLimitError
+
+try:
+  client.features() # receives response indicating a rate limit and retry time in seconds
+except RateLimitError as err:
+  # Handle the error -- define default behavior during rate limiting
+```
+
 ## Testing
 
 The Memory Adapter can be used for writing tests. This creates an ephemeral memory only implementation of the feature_gate client API. This is non-suitable for production only for tests.

feature_gate/clients/posthog_api_client.py

@@ -15,6 +15,9 @@
 class PosthogAPIClientError(Exception):
   pass
 
+class RateLimitError(Exception):
+  pass
+
 class PosthogAPIClient:
   def __init__(self, api_base=None, api_key=None, project_id=None):
     if api_base is None:
@@ -185,28 +188,39 @@ def _get_headers(self):
   def _check_status_ok(self, code):
     return code == 200 or code == 201
 
+  def _check_status_too_many_requests(self, code):
+    return code == 429
+
   def _map_single_response(self, method, path, response):
     ret = None
     if self._check_status_ok(response.status_code):
-        data = response.json()
-        self.logger.info("request successful", method=method, path=path, status_code=response.status_code, response=data)
-        ret = self._map_single_response_success(data)
+      data = response.json()
+      self.logger.info("request successful", method=method, path=path, status_code=response.status_code, response=data)
+      ret = self._map_single_response_success(data)
+    elif self._check_status_too_many_requests(response.status_code):
+      data = response.json()
+      self.logger.info("request failed", method=method, path=path, status_code=response.status_code, response=data)
+      raise RateLimitError(f"{data['detail']}")
     else:
-        data = response.json()
-        self.logger.info("request failed", method=method, path=path, status_code=response.status_code, response=data)
-        ret = self._map_error_response(response.status_code, data)
+      data = response.json()
+      self.logger.info("request failed", method=method, path=path, status_code=response.status_code, response=data)
+      ret = self._map_error_response(response.status_code, data)
     return ret
 
   def _map_list_response(self, method, path, response):
     ret = None
     if self._check_status_ok(response.status_code):
-        data = response.json()
-        self.logger.info("request successful", method=method, path=path, status_code=response.status_code, response=data)
-        ret = self._map_list_response_success(data)
+      data = response.json()
+      self.logger.info("request successful", method=method, path=path, status_code=response.status_code, response=data)
+      ret = self._map_list_response_success(data)
+    elif self._check_status_too_many_requests(response.status_code):
+      data = response.json()
+      self.logger.info("request failed", method=method, path=path, status_code=response.status_code, response=data)
+      raise RateLimitError(f"{data['detail']}")
     else:
-        data = response.json()
-        self.logger.info("request failed", method=method, path=path, status_code=response.status_code, response=data)
-        ret = self._map_error_response(response.status_code, data)
+      data = response.json()
+      self.logger.info("request failed", method=method, path=path, status_code=response.status_code, response=data)
+      ret = self._map_error_response(response.status_code, data)
     return ret
 
   def _map_error_response(self, code, data):
@@ -238,3 +252,7 @@ def _map_list_response_success(self, data):
   def _log_posthog_connection_error(self, error):
     self.logger.error(f"Posthog connection error - {error}")
     raise PosthogAPIClientError(f"Posthog connection error - {error}")
+
+  def _log_posthog_rate_limit_error(self, error):
+    self.logger.error(f"Posthog rate limit error - {error}")
+    raise RateLimitError(f"Posthog rate limit error error - {error}")

tests/feature_gate/adapters/posthog_test.py

@@ -2,9 +2,10 @@
 import requests
 
 from feature_gate.adapters.posthog import PosthogAdapter
+from feature_gate.clients.posthog_api_client import RateLimitError
 from feature_gate.client import Client, FeatureNotFound
 from feature_gate.feature import Feature
-from tests.fixtures.posthog_api_client.mocks import build_feature_from_mocks, mock_add_feature_funnel, mock_disable_feature_funnel, mock_enable_feature_funnel, mock_features_when_empty, mock_features_when_error_returned, mock_features_when_funnel, mock_funnel_is_disabled, mock_funnel_is_enabled, mock_remove_feature_funnel
+from tests.fixtures.posthog_api_client.mocks import build_feature_from_mocks, mock_add_feature_funnel, mock_disable_feature_funnel, mock_enable_feature_funnel, mock_features_when_empty, mock_features_when_error_returned, mock_features_when_funnel, mock_funnel_is_disabled, mock_funnel_is_enabled, mock_remove_feature_funnel, mock_rate_limiting_error
 from unittest.mock import patch
 
 def configured_client():
@@ -96,6 +97,15 @@ def test_is_enabled_raises_an_error_when_the_api_response_returns_an_error_statu
     except FeatureNotFound as e:
       assert str(e) == "Feature funnel_test not found"
 
+def test_is_enabled_raises_an_error_when_rate_limited():
+  client = configured_client()
+  feature = build_feature_from_mocks()
+  with patch.object(requests, 'get', return_value=mock_rate_limiting_error()):
+    try:
+      resp = client.is_enabled(feature.key)
+    except RateLimitError as e:
+      assert str(e) == "Request was throttled. Expected available in 5 seconds."
+
 def test_enable_returns_true_when_feature_exists():
   client = configured_client()
   feature = build_feature_from_mocks()

tests/fixtures/posthog_api_client/mocks.py

@@ -89,3 +89,11 @@ def mock_features_when_error_returned():
       return_value=load_response('get_features_when_empty')
     )
   )
+
+def mock_rate_limiting_error():
+  return Mock(
+    status_code=429,
+    json=Mock(
+      return_value=load_response('rate_limiting_error')
+    )
+  )

tests/fixtures/posthog_api_client/rate_limiting_error.json

@@ -0,0 +1,6 @@
+{
+  "type": "throttled_error",
+  "code": "throttled",
+  "detail": "Request was throttled. Expected available in 5 seconds.",
+  "attr": null
+}