diff --git a/CHANGELOG.md b/CHANGELOG.md index c98aa7dd..576ac3da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ Changelog ========= +## v4.6.2 (2024-03-05) + +### Bug fixes + +* Ensure sessions are sent at exit + [#371](https://github.com/bugsnag/bugsnag-python/pull/371) + ## v4.6.1 (2023-12-11) ### Bug fixes diff --git a/bugsnag/configuration.py b/bugsnag/configuration.py index df5604fc..54b732a1 100644 --- a/bugsnag/configuration.py +++ b/bugsnag/configuration.py @@ -273,6 +273,25 @@ def delivery(self): def delivery(self, value): if hasattr(value, 'deliver') and callable(value.deliver): self._delivery = value + + # deliver_sessions is _technically_ optional in that if you disable + # session tracking it will never be called + # this should be made mandatory in the next major release + if ( + hasattr(value, 'deliver_sessions') and + callable(value.deliver_sessions) + ): + parameter_names = value.deliver_sessions.__code__.co_varnames + + if 'options' not in parameter_names: + warnings.warn( + 'delivery.deliver_sessions should accept an ' + + '"options" parameter to allow for synchronous ' + + 'delivery, sessions may be lost when the process ' + + 'exits', + DeprecationWarning + ) + else: message = ('delivery should implement Delivery interface, got ' + '{0}. This will be an error in a future release.') diff --git a/bugsnag/delivery.py b/bugsnag/delivery.py index 491c276b..d2a02195 100644 --- a/bugsnag/delivery.py +++ b/bugsnag/delivery.py @@ -55,13 +55,13 @@ class Delivery: def __init__(self): self.sent_session_warning = False - def deliver(self, config, payload: Any, options={}): + def deliver(self, config, payload: Any, options=None): """ Sends error reports to Bugsnag """ pass - def deliver_sessions(self, config, payload: Any): + def deliver_sessions(self, config, payload: Any, options=None): """ Sends sessions to Bugsnag """ @@ -72,10 +72,12 @@ def deliver_sessions(self, config, payload: Any): 'No sessions will be sent to Bugsnag.') self.sent_session_warning = True else: - options = { - 'endpoint': config.session_endpoint, - 'success': 202, - } + if options is None: + options = {} + + options['endpoint'] = config.session_endpoint + options['success'] = 202 + self.deliver(config, payload, options) def queue_request(self, request: Callable, config, options: Dict): @@ -96,8 +98,9 @@ def safe_request(): class UrllibDelivery(Delivery): - - def deliver(self, config, payload: Any, options={}): + def deliver(self, config, payload: Any, options=None): + if options is None: + options = {} def request(): uri = options.pop('endpoint', config.endpoint) @@ -134,8 +137,9 @@ def request(): class RequestsDelivery(Delivery): - - def deliver(self, config, payload: Any, options={}): + def deliver(self, config, payload: Any, options=None): + if options is None: + options = {} def request(): uri = options.pop('endpoint', config.endpoint) diff --git a/bugsnag/notifier.py b/bugsnag/notifier.py index 91d4310b..2a6ac17e 100644 --- a/bugsnag/notifier.py +++ b/bugsnag/notifier.py @@ -1,5 +1,5 @@ _NOTIFIER_INFORMATION = { 'name': 'Python Bugsnag Notifier', 'url': 'https://github.com/bugsnag/bugsnag-python', - 'version': '4.6.1' + 'version': '4.6.2' } diff --git a/bugsnag/sessiontracker.py b/bugsnag/sessiontracker.py index 67d7d16b..ecf4dbcd 100644 --- a/bugsnag/sessiontracker.py +++ b/bugsnag/sessiontracker.py @@ -54,7 +54,7 @@ def start_session(self): _session_info.set(new_session) self.__queue_session(start_time) - def send_sessions(self): + def send_sessions(self, asynchronous=True): self.mutex.acquire() try: sessions = [] @@ -66,7 +66,8 @@ def send_sessions(self): self.session_counts = {} finally: self.mutex.release() - self.__deliver(sessions) + + self.__deliver(sessions, asynchronous) def __start_delivery(self): if self.delivery_thread is None: @@ -83,7 +84,8 @@ def deliver(): def cleanup(): if self.delivery_thread is not None: self.delivery_thread.cancel() - self.send_sessions() + + self.send_sessions(asynchronous=False) atexit.register(cleanup) @@ -96,7 +98,7 @@ def __queue_session(self, start_time: str): finally: self.mutex.release() - def __deliver(self, sessions: List[Dict]): + def __deliver(self, sessions: List[Dict], asynchronous=True): if not sessions: self.config.logger.debug("No sessions to deliver") return @@ -132,7 +134,19 @@ def __deliver(self, sessions: List[Dict]): ) encoded_payload = encoder.encode(payload) - self.config.delivery.deliver_sessions(self.config, encoded_payload) + + deliver = self.config.delivery.deliver_sessions + + if 'options' in deliver.__code__.co_varnames: + deliver( + self.config, + encoded_payload, + options={'asynchronous': asynchronous} + ) + else: + deliver(self.config, encoded_payload) + + except Exception as e: self.config.logger.exception('Sending sessions failed %s', e) diff --git a/setup.py b/setup.py index a885b8f1..fb791d0e 100755 --- a/setup.py +++ b/setup.py @@ -14,7 +14,7 @@ setup( name='bugsnag', - version='4.6.1', + version='4.6.2', description='Automatic error monitoring for django, flask, etc.', long_description=__doc__, author='Simon Maynard', diff --git a/tests/integrations/test_asgi.py b/tests/integrations/test_asgi.py index 8feec126..47441a5c 100644 --- a/tests/integrations/test_asgi.py +++ b/tests/integrations/test_asgi.py @@ -139,7 +139,6 @@ async def app(scope, recv, send): self.assertEqual('GET', request['httpMethod']) self.assertEqual('http', request['type']) self.assertEqual('http://testserver/', request['url']) - self.assertEqual('testclient', request['clientIp']) self.assertEqual('testclient', request['headers']['user-agent']) exception = payload['events'][0]['exceptions'][0] diff --git a/tests/test_configuration.py b/tests/test_configuration.py index d8cb82bb..03266940 100644 --- a/tests/test_configuration.py +++ b/tests/test_configuration.py @@ -195,6 +195,13 @@ class BadDelivery(object): def deliv(self, *args, **kwargs): pass + class OkDelivery: + def deliver(self, *args, **kwargs): + pass + + def deliver_sessions(self, config, payload): + pass + class GoodDelivery(object): def deliver(self, *args, **kwargs): pass @@ -213,6 +220,20 @@ def deliver(self, *args, **kwargs): assert len(record) == 1 assert c.delivery == good + with pytest.warns(DeprecationWarning) as record: + ok = OkDelivery() + + c.configure(delivery=ok) + + assert len(record) == 1 + assert str(record[0].message) == ( + 'delivery.deliver_sessions should accept an "options" ' + + 'parameter to allow for synchronous delivery, sessions ' + + 'may be lost when the process exits' + ) + + assert c.delivery == ok + def test_validate_hostname(self): c = Configuration() with pytest.warns(RuntimeWarning) as record: