From 4ddd3c2637aee52a39a70dae88f163024fee7699 Mon Sep 17 00:00:00 2001
From: Owen W. Taylor <otaylor@fishsoup.net>
Date: Apr 26 2022 21:12:16 +0000
Subject: test_build: exit rather than hanging on event handler exception


Event handlers decorated with @celery_app_task don't raise an exception -
they just log the exception, leaving the Moksha hub running. This meant
that any failures in test_build would result in the test suite hanging
rather than failing usefully.

We solve this by adding a new class EventTrap which acts as a context
manager. Any exceptions that occur in event handlers are set on the
current EventTrap, and the test_build tests re-raise the exception.

---

diff --git a/module_build_service/scheduler/events.py b/module_build_service/scheduler/events.py
index b02dc7b..bb56c04 100644
--- a/module_build_service/scheduler/events.py
+++ b/module_build_service/scheduler/events.py
@@ -15,6 +15,7 @@ however Brew sends to topic brew.build.complete, etc.
 from __future__ import absolute_import
 from functools import wraps
 import sched
+import threading
 import time
 
 from module_build_service.common import log
@@ -66,6 +67,45 @@ class Scheduler(sched.scheduler):
 scheduler = Scheduler(time.time, delayfunc=lambda x: x)
 
 
+class EventTrap():
+    """
+    A context handler that can be used to provide a place to store exceptions
+    that occur in event handlers during a section of code. This is global rather
+    than per-thread, because we we set up the trap in the main thread, but
+    event event handlers are processed in the thread where moksha runs MBSConsumer.
+
+    This is needed because @celery_app.task simply logs and ignores exceptions.
+    """
+    lock = threading.Lock()
+    current = None
+
+    def __init__(self):
+        self.exception = None
+
+    def __enter__(self):
+        with EventTrap.lock:
+            EventTrap.current = self
+            return self
+
+    def __exit__(self, type, value, tb):
+        with EventTrap.lock:
+            EventTrap.current = None
+
+    @classmethod
+    def set_exception(cls, exception):
+        with cls.lock:
+            if cls.current and not cls.current.exception:
+                cls.current.exception = exception
+
+    @classmethod
+    def get_exception(cls):
+        with cls.lock:
+            if cls.current:
+                return cls.current.exception
+            else:
+                return None
+
+
 def mbs_event_handler(func):
     """
     A decorator for MBS event handlers. It implements common tasks which should otherwise
@@ -77,6 +117,9 @@ def mbs_event_handler(func):
     def wrapper(*args, **kwargs):
         try:
             return func(*args, **kwargs)
+        except Exception as e:
+            EventTrap.set_exception(e)
+            raise e
         finally:
             scheduler.run()
     # save origin function as functools.wraps from python2 doesn't preserve the signature
diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py
index 1f8c515..a7e3a3f 100644
--- a/tests/test_build/test_build.py
+++ b/tests/test_build/test_build.py
@@ -83,6 +83,16 @@ def make_simple_stop_condition():
     return stop_condition
 
 
+def make_trapped_stop_condition(stop_condition):
+    def trapped_stop_condition(message):
+        if events.EventTrap.get_exception():
+            return True
+
+        return stop_condition(message)
+
+    return trapped_stop_condition
+
+
 def main(initial_messages, stop_condition):
     """ Run the consumer until some condition is met.
 
@@ -108,18 +118,25 @@ def main(initial_messages, stop_condition):
 
     consumers = [module_build_service.scheduler.consumer.MBSConsumer]
 
-    # Note that the hub we kick off here cannot send any message.  You
-    # should use fedmsg.publish(...) still for that.
-    moksha.hub.main(
-        # Pass in our config dict
-        options=config,
-        # Only run the specified consumers if any are so specified.
-        consumers=consumers,
-        # Do not run default producers.
-        producers=[],
-        # Tell moksha to quiet its logging.
-        framework=False,
-    )
+    # The events.EventTrap context handler allows us to detect exceptions
+    # in event handlers and re-raise them here so that tests fail usefully
+    # rather than just hang.
+    with events.EventTrap() as trap:
+        # Note that the hub we kick off here cannot send any message.  You
+        # should use fedmsg.publish(...) still for that.
+        moksha.hub.main(
+            # Pass in our config dict
+            options=config,
+            # Only run the specified consumers if any are so specified.
+            consumers=consumers,
+            # Do not run default producers.
+            producers=[],
+            # Tell moksha to quiet its logging.
+            framework=False,
+        )
+
+        if trap.exception:
+            raise trap.exception
 
 
 class FakeSCM(object):
@@ -388,9 +405,10 @@ def cleanup_moksha():
 class BaseTestBuild:
 
     def run_scheduler(self, msgs=None, stop_condition=None):
+        stop_condition = stop_condition or make_simple_stop_condition()
         main(
             msgs or [],
-            stop_condition or make_simple_stop_condition()
+            make_trapped_stop_condition(stop_condition)
         )