Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v0.29.2 - 🚩 [Bug Fix] error boundary synchronously sends request Latest #22

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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 @@ -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,
Expand All @@ -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={
Expand All @@ -101,6 +117,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 @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion statsig/version.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = '0.29.1'
__version__ = '0.29.2'
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
Loading