Skip to content

Commit

Permalink
fix: make error boundary non-blocking (#246)
Browse files Browse the repository at this point in the history
  • Loading branch information
tore-statsig authored Apr 29, 2024
1 parent 74d4197 commit 23c9e00
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 30 deletions.
31 changes: 24 additions & 7 deletions statsig/statsig_error_boundary.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from concurrent.futures import ThreadPoolExecutor
import traceback
import requests
from .statsig_errors import StatsigNameError, StatsigRuntimeError, StatsigValueError
Expand All @@ -6,8 +7,7 @@
from .diagnostics import Diagnostics, Key, Context, Marker
from . import globals

REQUEST_TIMEOUT = 20

REQUEST_TIMEOUT = 5

class _StatsigErrorBoundary:
endpoint = "https://statsigapi.net/v1/sdk_exception"
Expand All @@ -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
Expand Down Expand Up @@ -58,6 +59,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,
Expand All @@ -81,18 +85,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={
Expand All @@ -104,6 +120,7 @@ def log_exception(
timeout=REQUEST_TIMEOUT,
)
except BaseException:
# no-op, best effort
pass

def _start_diagnostics(self, key, configName):
Expand Down
1 change: 1 addition & 0 deletions statsig/statsig_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,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)
Expand Down
3 changes: 3 additions & 0 deletions tests/test_statsig_error_boundary.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@ 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):
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):
Expand Down Expand Up @@ -164,6 +166,7 @@ def recover():
return err

def _get_requests(self):
self._boundary.shutdown(True)
return TestStatsigErrorBoundary.requests


Expand Down
48 changes: 25 additions & 23 deletions tests/test_statsig_error_boundary_usage.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ def mocked_post(*args, **kwargs):
})


def _get_requests():
def _get_requests(statsig):
statsig._errorBoundary.shutdown(True)
return TestStatsigErrorBoundaryUsage.requests


Expand Down Expand Up @@ -50,26 +51,27 @@ 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)

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, {})
Expand All @@ -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, {})
Expand All @@ -88,52 +90,52 @@ 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")

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": {}
Expand Down

0 comments on commit 23c9e00

Please sign in to comment.