From 127ee15fdd2b2cf3dcfb7149f9cb2ead94feaf7a Mon Sep 17 00:00:00 2001 From: tore-statsig <74584483+tore-statsig@users.noreply.github.com> Date: Mon, 29 Apr 2024 11:25:04 -0700 Subject: [PATCH 1/2] fix: make error boundary non-blocking (#246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ![Screenshot 2024-04-26 at 3 49 03 PM](https://github.com/statsig-io/private-python-sdk/assets/74584483/7ce98358-2cd8-4ade-921b-9e91ccc4af9a) --- statsig/statsig_error_boundary.py | 31 ++++++++++---- statsig/statsig_server.py | 1 + tests/test_statsig_error_boundary.py | 3 ++ tests/test_statsig_error_boundary_usage.py | 48 +++++++++++----------- 4 files changed, 53 insertions(+), 30 deletions(-) diff --git a/statsig/statsig_error_boundary.py b/statsig/statsig_error_boundary.py index b8a9351..d1a95cf 100644 --- a/statsig/statsig_error_boundary.py +++ b/statsig/statsig_error_boundary.py @@ -1,3 +1,4 @@ +from concurrent.futures import ThreadPoolExecutor import traceback import requests from .statsig_errors import StatsigNameError, StatsigRuntimeError, StatsigValueError @@ -6,8 +7,7 @@ from .diagnostics import Diagnostics, Key, Context from . import globals -REQUEST_TIMEOUT = 20 - +REQUEST_TIMEOUT = 5 class _StatsigErrorBoundary: endpoint = "https://statsigapi.net/v1/sdk_exception" @@ -17,6 +17,7 @@ class _StatsigErrorBoundary: def __init__(self, is_silent=False): self._seen = set() self._is_silent = is_silent + self._executor = ThreadPoolExecutor(max_workers=1) def set_statsig_options_and_metadata( self, statsig_options: StatsigOptions, statsig_metadata: dict @@ -55,6 +56,9 @@ def empty_recover(): self.capture(tag, task, empty_recover) + def shutdown(self, wait=False): + self._executor.shutdown(wait) + def log_exception( self, tag: str, @@ -78,18 +82,30 @@ def log_exception( if bypass_dedupe is False and name in self._seen: return self._seen.add(name) + + self._executor.submit( + self._post_exception, + name, + traceback.format_exc(), + tag, + extra, + ) + except BaseException: + # no-op, best effort + pass + + def _post_exception(self, name, info, tag, extra): + try: requests.post( self.endpoint, json={ - "exception": type(exception).__name__, - "info": traceback.format_exc(), + "exception": name, + "info": info, "statsigMetadata": self._metadata, "tag": tag, "extra": extra, "statsigOptions": ( - self._options.get_logging_copy() - if isinstance(self._options, StatsigOptions) - else None + self._options.get_logging_copy() if isinstance(self._options, StatsigOptions) else None ), }, headers={ @@ -101,6 +117,7 @@ def log_exception( timeout=REQUEST_TIMEOUT, ) except BaseException: + # no-op, best effort pass def _start_diagnostics(self, key, configName): diff --git a/statsig/statsig_server.py b/statsig/statsig_server.py index f530418..9514dbd 100644 --- a/statsig/statsig_server.py +++ b/statsig/statsig_server.py @@ -301,6 +301,7 @@ def task(): self.__shutdown_event.set() self._logger.shutdown() self._spec_store.shutdown() + self._errorBoundary.shutdown() self._initialized = False self._errorBoundary.swallow("shutdown", task) diff --git a/tests/test_statsig_error_boundary.py b/tests/test_statsig_error_boundary.py index a332ab2..6f1b42d 100644 --- a/tests/test_statsig_error_boundary.py +++ b/tests/test_statsig_error_boundary.py @@ -42,6 +42,7 @@ def recover(): called = True self._boundary.capture("", task, recover) + self._boundary.shutdown(True) self.assertTrue(called) def test_has_default_recovery_of_none(self, mock_post): @@ -49,6 +50,7 @@ def task(): raise RuntimeError() res = self._boundary.swallow("", task) + self._boundary.shutdown(True) self.assertIsNone(res) def test_logging_to_correct_endpoint(self, mock_post): @@ -164,6 +166,7 @@ def recover(): return err def _get_requests(self): + self._boundary.shutdown(True) return TestStatsigErrorBoundary.requests diff --git a/tests/test_statsig_error_boundary_usage.py b/tests/test_statsig_error_boundary_usage.py index ddfcd7d..3160feb 100644 --- a/tests/test_statsig_error_boundary_usage.py +++ b/tests/test_statsig_error_boundary_usage.py @@ -16,7 +16,8 @@ def mocked_post(*args, **kwargs): }) -def _get_requests(): +def _get_requests(statsig): + statsig._errorBoundary.shutdown(True) return TestStatsigErrorBoundaryUsage.requests @@ -50,9 +51,10 @@ def test_errors_with_initialize(self, mock_post): statsig = StatsigServer() TestStatsigErrorBoundaryUsage.requests = [] statsig.initialize("secret-key", "_BAD_OPTIONS_") + - self.assertEqual(len(_get_requests()), 1) - trace = _get_requests()[0]['body']['info'] + self.assertEqual(len(_get_requests(statsig)), 1) + trace = _get_requests(self._instance)[0]['body']['info'] self.assertIn( "AttributeError: 'str' object has no attribute 'api'", trace) self.assertTrue(statsig._initialized) @@ -60,16 +62,16 @@ def test_errors_with_initialize(self, mock_post): def test_errors_with_check_gate(self, mock_post): res = self._instance.check_gate(self._user, "a_gate") - self.assertEqual(len(_get_requests()), 1) - trace = _get_requests()[0]['body']['info'] + self.assertEqual(len(_get_requests(self._instance)), 1) + trace = _get_requests(self._instance)[0]['body']['info'] self.assertIn('object has no attribute \'check_gate\'\n', trace) self.assertFalse(res) def test_errors_with_get_config(self, mock_post): res = self._instance.get_config(self._user, "a_config") - self.assertEqual(len(_get_requests()), 1) - trace = _get_requests()[0]['body']['info'] + self.assertEqual(len(_get_requests(self._instance)), 1) + trace = _get_requests(self._instance)[0]['body']['info'] self.assertIn('object has no attribute \'get_config\'\n', trace) self.assertIsInstance(res, DynamicConfig) self.assertEqual(res.value, {}) @@ -78,8 +80,8 @@ def test_errors_with_get_config(self, mock_post): def test_errors_with_get_experiment(self, mock_post): res = self._instance.get_experiment(self._user, "an_experiment") - self.assertEqual(len(_get_requests()), 1) - trace = _get_requests()[0]['body']['info'] + self.assertEqual(len(_get_requests(self._instance)), 1) + trace = _get_requests(self._instance)[0]['body']['info'] self.assertIn('object has no attribute \'get_config\'\n', trace) self.assertIsInstance(res, DynamicConfig) self.assertEqual(res.value, {}) @@ -88,8 +90,8 @@ def test_errors_with_get_experiment(self, mock_post): def test_errors_with_get_layer(self, mock_post): res = self._instance.get_layer(self._user, "a_layer") - self.assertEqual(len(_get_requests()), 1) - trace = _get_requests()[0]['body']['info'] + self.assertEqual(len(_get_requests(self._instance)), 1) + trace = _get_requests(self._instance)[0]['body']['info'] self.assertIn('object has no attribute \'get_layer\'\n', trace) self.assertIsInstance(res, Layer) self.assertEqual(res.name, "a_layer") @@ -97,43 +99,43 @@ def test_errors_with_get_layer(self, mock_post): def test_errors_with_log_event(self, mock_post): self._instance.log_event(StatsigEvent(self._user, "an_event")) - self.assertEqual(len(_get_requests()), 1) - trace = _get_requests()[0]['body']['info'] + self.assertEqual(len(_get_requests(self._instance)), 1) + trace = _get_requests(self._instance)[0]['body']['info'] self.assertIn('object has no attribute \'log\'\n', trace) def test_errors_with_shutdown(self, mock_post): self._instance.shutdown() - self.assertEqual(len(_get_requests()), 1) - trace = _get_requests()[0]['body']['info'] + self.assertEqual(len(_get_requests(self._instance)), 1) + trace = _get_requests(self._instance)[0]['body']['info'] self.assertIn('object has no attribute \'shutdown\'\n', trace) def test_errors_with_override_gate(self, mock_post): self._instance.override_gate("a_gate", False) - self.assertEqual(len(_get_requests()), 1) - trace = _get_requests()[0]['body']['info'] + self.assertEqual(len(_get_requests(self._instance)), 1) + trace = _get_requests(self._instance)[0]['body']['info'] self.assertIn('object has no attribute \'override_gate\'\n', trace) def test_errors_with_override_config(self, mock_post): self._instance.override_config("a_config", {}) - self.assertEqual(len(_get_requests()), 1) - trace = _get_requests()[0]['body']['info'] + self.assertEqual(len(_get_requests(self._instance)), 1) + trace = _get_requests(self._instance)[0]['body']['info'] self.assertIn('object has no attribute \'override_config\'\n', trace) def test_errors_with_override_experiment(self, mock_post): self._instance.override_experiment("an_experiment", {}) - self.assertEqual(len(_get_requests()), 1) - trace = _get_requests()[0]['body']['info'] + self.assertEqual(len(_get_requests(self._instance)), 1) + trace = _get_requests(self._instance)[0]['body']['info'] self.assertIn('object has no attribute \'override_config\'\n', trace) def test_errors_with_evaluate_all(self, mock_post): res = self._instance.evaluate_all(self._user) - self.assertEqual(len(_get_requests()), 1) - trace = _get_requests()[0]['body']['info'] + self.assertEqual(len(_get_requests(self._instance)), 1) + trace = _get_requests(self._instance)[0]['body']['info'] self.assertIn('object has no attribute \'get_all_gates\'\n', trace) self.assertEqual(res, { "feature_gates": {}, "dynamic_configs": {} From 286db54f72ec6eeb2a2dc77f0cad64761227dc0e Mon Sep 17 00:00:00 2001 From: tore-statsig <74584483+tore-statsig@users.noreply.github.com> Date: Mon, 29 Apr 2024 12:51:56 -0700 Subject: [PATCH 2/2] v0.29.2 - issue error boundary requests in the background --- statsig/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/statsig/version.py b/statsig/version.py index 5bb330e..bcf7f7e 100644 --- a/statsig/version.py +++ b/statsig/version.py @@ -1 +1 @@ -__version__ = '0.29.1' +__version__ = '0.29.2'