fix(api): stop transient DB errors from 500ing throttled requests

posthog/rate_limit.py

@@ -43,6 +43,12 @@
     labelnames=[LABEL_TEAM_ID, LABEL_PATH, LABEL_ROUTE],
 )
 
+RATE_LIMIT_FAIL_OPEN_COUNTER = Counter(
+    "rate_limit_fail_open_total",
+    "Requests allowed through without rate-limiting because checking the limit raised an exception.",
+    labelnames=["scope", "exception"],
+)
+
 
 @lru_cache(maxsize=1)
 def get_team_allow_list(_ttl: int) -> list[str]:
@@ -67,7 +73,15 @@ def is_rate_limit_enabled(_ttl: int) -> bool:
     The setting will change way less frequently than it will be called
     _ttl is passed an infrequently changing value to ensure the cache is invalidated after some delay
     """
-    return get_instance_setting("RATE_LIMIT_ENABLED")
+    try:
+        return get_instance_setting("RATE_LIMIT_ENABLED")
+    except Exception as e:
+        # This used to propagate straight through allow_request (called outside any
+        # try/except there), turning a transient DB blip into a 500 on every throttled
+        # request instead of the fail-open behavior the rest of this file uses.
+        capture_exception(e)
+        RATE_LIMIT_FAIL_OPEN_COUNTER.labels(scope="is_rate_limit_enabled", exception=type(e).__name__).inc()
+        return False
 
 
 path_by_env_pattern = re.compile(r"^/api/environments/(\d+)/")
@@ -236,6 +250,7 @@ def _allow_request_internal(self, request, view, *, personal_api_key_only: bool)
             return False
         except Exception as e:
             capture_exception(e)
+            RATE_LIMIT_FAIL_OPEN_COUNTER.labels(scope=self.scope, exception=type(e).__name__).inc()
             return True
 
     def allow_request(self, request, view):
@@ -1244,6 +1259,7 @@ def allow_request(self, request, view):
             return True
         except Exception as e:
             capture_exception(e)
+            RATE_LIMIT_FAIL_OPEN_COUNTER.labels(scope=self.scope, exception=type(e).__name__).inc()
             return True
 
 

posthog/test/test_rate_limit.py

@@ -37,6 +37,25 @@
 )
 
 
+class TestIsRateLimitEnabled(SimpleTestCase):
+    def setUp(self) -> None:
+        rate_limit.is_rate_limit_enabled.cache_clear()
+
+    def tearDown(self) -> None:
+        rate_limit.is_rate_limit_enabled.cache_clear()
+
+    @patch("posthog.rate_limit.capture_exception")
+    @patch("posthog.rate_limit.get_instance_setting", side_effect=Exception("cache lookup failed for function 481"))
+    def test_fails_open_when_the_instance_setting_lookup_raises(
+        self, _get_instance_setting: Mock, capture_exception_mock: Mock
+    ) -> None:
+        # Regression guard: this call happens outside the try/except in allow_request,
+        # so an unhandled exception here used to 500 every throttled request instead
+        # of falling back to "rate limiting disabled".
+        self.assertFalse(rate_limit.is_rate_limit_enabled(0))
+        capture_exception_mock.assert_called_once()
+
+
 class TestUserAPI(APIBaseTest):
     def setUp(self):
         super().setUp()