Ensure sessions are sent at exit

CHANGELOG.md

@@ -1,6 +1,13 @@
 Changelog
 =========
 
+## TBD
+
+### 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

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.')

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)

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)
 

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: