Repository: bluesky/bluesky
Pull request #1525 (2022-06-18 00:31:11)

FIX: make trigger and read careful
## Description

Previously we would get confusing errors when:

  - event had been opened
  - read or describe raise any errors once
  - a wrapper (such as the baseline pre-processor) tries to take a reading as
  part of plan clean up

The error would be that you could not open an event with an event open which
would effectively drop the original error (the one you actually need to debug
the issue!) on the floor.

This could also be fixed by improving the multi-exception handling in the RE,
but this is a simpler fix.


## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->

These errors are among the most confusing.  I helped @maffettone debug one of these today and finally got my act together to write this patch.

The fix is to make sure that `trigger_and_read` never leaves an event "open" when it returns control to its caller.


## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->

Added a test.


Patch:
diff --git a/bluesky/plan_stubs.py b/bluesky/plan_stubs.py
index eef58a96..97349f78 100644
--- a/bluesky/plan_stubs.py
+++ b/bluesky/plan_stubs.py
@@ -887,6 +887,7 @@ def trigger_and_read(devices, name='primary'):
     msg : Msg
         messages to 'trigger', 'wait' and 'read'
     """
+    from .preprocessors import contingency_wrapper
     # If devices is empty, don't emit 'create'/'save' messages.
     if not devices:
         yield from null()
@@ -904,13 +905,29 @@ def trigger_and_read(devices, name='primary'):
         if not no_wait:
             yield from wait(group=grp)
         yield from create(name)
-        ret = {}  # collect and return readings to give plan access to them
-        for obj in devices:
-            reading = (yield from read(obj))
-            if reading is not None:
-                ret.update(reading)
-        yield from save()
+
+        def read_plan():
+            ret = {}  # collect and return readings to give plan access to them
+            for obj in devices:
+                reading = (yield from read(obj))
+                if reading is not None:
+                    ret.update(reading)
+            return ret
+
+        def standard_path():
+            yield from save()
+
+        def exception_path(exp):
+            yield from drop()
+            raise exp
+
+        ret = yield from contingency_wrapper(
+            read_plan(),
+            except_plan=exception_path,
+            else_plan=standard_path
+            )
         return ret
+
     from .preprocessors import rewindable_wrapper
     return (yield from rewindable_wrapper(inner_trigger_and_read(),
                                           rewindable))


Test patch:
diff --git a/bluesky/tests/test_plans.py b/bluesky/tests/test_plans.py
index a97087bc..c7e9e7c1 100644
--- a/bluesky/tests/test_plans.py
+++ b/bluesky/tests/test_plans.py
@@ -4,6 +4,7 @@ import inspect
 from bluesky.tests.utils import DocCollector
 import bluesky.plans as bp
 import bluesky.plan_stubs as bps
+import bluesky.preprocessors as bpp
 import numpy as np
 import numpy.testing as npt
 import pandas as pd
@@ -657,3 +658,59 @@ def test_grid_scans_failing(RE, hw, plan):
                     hw.motor1, 4, 5, 6,
                     hw.motor2, 7, 8, 9)
             RE(plan([hw.det], *args, snake_axes=snake_axes))
+
+
+def test_describe_failure(RE):
+    ophyd = pytest.importorskip("ophyd")
+
+    class Aardvark(Exception):
+        # Unique exception to test behavior of describe.
+        ...
+
+    class BadSignalDescribe(ophyd.Signal):
+        def describe(self):
+            raise Aardvark("Look, an aardvark!")
+
+    class BadSignalRead(ophyd.Signal):
+        def read(self):
+            raise Aardvark("Look, the other aardvark!")
+
+    bad_signal1 = BadSignalDescribe(value=5, name="Arty")
+    bad_signal2 = BadSignalRead(value=5, name="Arty")
+    good_signal = ophyd.Signal(value=42, name='baseline')
+
+    class StreamTester:
+        def __init__(self):
+            self.event_count = 0
+            self.stream_names = set()
+
+        def __call__(self, name, doc):
+            if name == 'event':
+                self.event_count += 1
+            if name == 'descriptor':
+                self.stream_names.add(doc['name'])
+
+        def verify(self):
+            assert self.event_count == 2
+            assert self.stream_names == set(['baseline'])
+    st = StreamTester()
+    with pytest.raises(Aardvark, match="Look, an aardvark!"):
+        RE(
+            bpp.baseline_wrapper(
+                bp.count([bad_signal1]),
+                [good_signal]
+            ),
+            st
+        )
+    st.verify()
+
+    st = StreamTester()
+    with pytest.raises(Aardvark, match="Look, the other aardvark!"):
+        RE(
+            bpp.baseline_wrapper(
+                bp.count([bad_signal2]),
+                [good_signal]
+            ),
+            st
+        )
+    st.verify()