Prevent multiple faulting threads (#26)

# Why

During debugging AiQ bugs in Python we discovered, the fingerprint is
always 0s. After a deeper investigation, I noted our algorithm can set
two faulting threads. This pull request fixes the problem and makes sure
we only set one faulting thread

---------

Co-authored-by: Konrad Dysput <[email protected]>

backtracepython/report.py

@@ -13,10 +13,13 @@
 class BacktraceReport:
     def __init__(self):
         self.fault_thread = threading.current_thread()
+        self.faulting_thread_id = str(self.fault_thread.ident)
         self.source_code = {}
         self.source_path_dict = {}
         self.attachments = []
 
+        stack_trace = self.__generate_stack_trace()
+
         attributes, annotations = attribute_manager.get()
         attributes.update({"error.type": "Exception"})
 
@@ -29,16 +32,19 @@ def __init__(self):
             "langVersion": python_version,
             "agent": "backtrace-python",
             "agentVersion": version_string,
-            "mainThread": str(self.fault_thread.ident),
+            "mainThread": self.faulting_thread_id,
             "attributes": attributes,
             "annotations": annotations,
-            "threads": self.generate_stack_trace(),
+            "threads": stack_trace,
         }
 
     def set_exception(self, garbage, ex_value, ex_traceback):
         self.report["classifiers"] = [ex_value.__class__.__name__]
         self.report["attributes"]["error.message"] = str(ex_value)
 
+        # reset faulting thread id and make sure the faulting thread is not listed twice
+        self.report["threads"][self.faulting_thread_id]["fault"] = False
+
         # update faulting thread with information from the error
         fault_thread_id = str(self.fault_thread.ident)
         if not fault_thread_id in self.report["threads"]:
@@ -50,42 +56,92 @@ def set_exception(self, garbage, ex_value, ex_traceback):
 
         faulting_thread = self.report["threads"][fault_thread_id]
 
-        faulting_thread["stack"] = self.convert_stack_trace(
-            self.traverse_exception_stack(ex_traceback), False
+        faulting_thread["stack"] = self.__convert_stack_trace(
+            self.__traverse_exception_stack(ex_traceback), False
         )
         faulting_thread["fault"] = True
+        self.faulting_thread_id = fault_thread_id
+        self.report["mainThread"] = self.faulting_thread_id
+
+    def capture_last_exception(self):
+        self.set_exception(*sys.exc_info())
+
+    def set_attribute(self, key, value):
+        self.report["attributes"][key] = value
+
+    def set_dict_attributes(self, target_dict):
+        self.report["attributes"].update(target_dict)
+
+    def set_annotation(self, key, value):
+        self.report["annotations"][key] = value
+
+    def get_annotations(self):
+        return self.report["annotations"]
+
+    def get_attributes(self):
+        return self.report["attributes"]
+
+    def set_dict_annotations(self, target_dict):
+        self.report["annotations"].update(target_dict)
+
+    def log(self, line):
+        self.log_lines.append(
+            {
+                "ts": time.time(),
+                "msg": line,
+            }
+        )
+
+    def add_attachment(self, attachment_path):
+        self.attachments.append(attachment_path)
+
+    def get_attachments(self):
+        return self.attachments
+
+    def get_data(self):
+        return self.report
+
+    def send(self):
+        if len(self.log_lines) != 0 and "Log" not in self.report["annotations"]:
+            self.report["annotations"]["Log"] = self.log_lines
+        from backtracepython.client import send
+
+        send(self)
 
-    def generate_stack_trace(self):
+    def __generate_stack_trace(self):
         current_frames = sys._current_frames()
         threads = {}
         for thread in threading.enumerate():
             thread_frame = current_frames.get(thread.ident)
             is_main_thread = thread.name == "MainThread"
-            threads[str(thread.ident)] = {
+            thread_id = str(thread.ident)
+            threads[thread_id] = {
                 "name": thread.name,
-                "stack": self.convert_stack_trace(
-                    self.traverse_process_thread_stack(thread_frame), is_main_thread
+                "stack": self.__convert_stack_trace(
+                    self.__traverse_process_thread_stack(thread_frame), is_main_thread
                 ),
                 "fault": is_main_thread,
             }
+            if is_main_thread:
+                self.faulting_thread_id = thread_id
 
         return threads
 
-    def traverse_exception_stack(self, traceback):
+    def __traverse_exception_stack(self, traceback):
         stack = []
         while traceback:
             stack.append({"frame": traceback.tb_frame, "line": traceback.tb_lineno})
             traceback = traceback.tb_next
         return reversed(stack)
 
-    def traverse_process_thread_stack(self, thread_frame):
+    def __traverse_process_thread_stack(self, thread_frame):
         stack = []
         while thread_frame:
             stack.append({"frame": thread_frame, "line": thread_frame.f_lineno})
             thread_frame = thread_frame.f_back
         return stack
 
-    def convert_stack_trace(self, thread_stack_trace, skip_backtrace_module):
+    def __convert_stack_trace(self, thread_stack_trace, skip_backtrace_module):
         stack_trace = []
 
         for thread_stack_frame in thread_stack_trace:
@@ -109,48 +165,3 @@ def convert_stack_trace(self, thread_stack_trace, skip_backtrace_module):
             )
 
         return stack_trace
-
-    def capture_last_exception(self):
-        self.set_exception(*sys.exc_info())
-
-    def set_attribute(self, key, value):
-        self.report["attributes"][key] = value
-
-    def set_dict_attributes(self, target_dict):
-        self.report["attributes"].update(target_dict)
-
-    def set_annotation(self, key, value):
-        self.report["annotations"][key] = value
-
-    def get_annotations(self):
-        return self.report["annotations"]
-
-    def get_attributes(self):
-        return self.report["attributes"]
-
-    def set_dict_annotations(self, target_dict):
-        self.report["annotations"].update(target_dict)
-
-    def log(self, line):
-        self.log_lines.append(
-            {
-                "ts": time.time(),
-                "msg": line,
-            }
-        )
-
-    def add_attachment(self, attachment_path):
-        self.attachments.append(attachment_path)
-
-    def get_attachments(self):
-        return self.attachments
-
-    def get_data(self):
-        return self.report
-
-    def send(self):
-        if len(self.log_lines) != 0 and "Log" not in self.report["annotations"]:
-            self.report["annotations"]["Log"] = self.log_lines
-        from backtracepython.client import send
-
-        send(self)

tests/test_stack_trace_parser.py

@@ -44,6 +44,39 @@ def test_main_thread_generation_with_exception():
         assert len(stack_trace["stack"]) == expected_number_of_frames
 
 
+def test_stack_trace_generation_from_background_thread():
+    background_thread_name = "test_background"
+    data_container = []
+
+    def throw_in_background():
+        try:
+            failing_function()
+        except:
+            report = BacktraceReport()
+            report.capture_last_exception()
+            data = report.get_data()
+            data_container.append(data)
+
+    thread = threading.Thread(target=throw_in_background, name=background_thread_name)
+    thread.start()
+    thread.join()
+    if data_container:
+        data = data_container[0]
+        faulting_thread = data["threads"][data["mainThread"]]
+        assert faulting_thread["name"] != "MainThread"
+        assert faulting_thread["name"] == background_thread_name
+        assert faulting_thread["fault"] == True
+        # make sure other threads are not marked as faulting threads
+        for thread_id in data["threads"]:
+            thread = data["threads"][thread_id]
+            if thread["name"] == background_thread_name:
+                continue
+            assert thread["fault"] == False
+
+    else:
+        assert False
+
+
 def test_background_thread_stack_trace_generation():
     if_stop = False