Improve the error messages by including command sent (#94)

src/pandablocks/_exchange.py

@@ -16,16 +16,27 @@ def __init__(self, to_send: Union[str, list[str]]):
         self.received: list[str] = []
         self.is_multiline = False
 
+    def _error_message(self) -> str:
+        sent = "\n".join(self.to_send)
+        received = "\n".join(self.received)
+        return f"{sent!r} -> {received!r}"
+
+    def check_ok(self):
+        assert self.received == ["OK"], self._error_message()
+
     @property
     def line(self) -> str:
         """Check received is not multiline and return the line"""
-        assert not self.is_multiline
-        return self.received[0]
+        assert not self.is_multiline and self.received[0].startswith(
+            "OK ="
+        ), self._error_message()
+        # Remove the OK= header
+        return self.received[0][4:]
 
     @property
     def multiline(self) -> list[str]:
         """Return the multiline received lines, processed to remove markup"""
-        assert self.is_multiline
+        assert self.is_multiline, self._error_message()
         # Remove the ! and . markup
         return [line[1:] for line in self.received[:-1]]
 

src/pandablocks/commands.py

@@ -207,10 +207,7 @@ def execute(self) -> ExchangeGenerator[Union[str, list[str]]]:
         if ex.is_multiline:
             return ex.multiline
         else:
-            # We got OK =value
-            line = ex.line
-            assert line.startswith("OK =")
-            return line[4:]
+            return ex.line
 
 
 @dataclass
@@ -232,10 +229,7 @@ class GetLine(Command[str]):
     def execute(self) -> ExchangeGenerator[str]:
         ex = Exchange(f"{self.field}?")
         yield ex
-        # Expect "OK =value"
-        line = ex.line
-        assert line.startswith("OK =")
-        return line[4:]
+        return ex.line
 
 
 @dataclass
@@ -285,7 +279,7 @@ def execute(self) -> ExchangeGenerator[None]:
         else:
             ex = Exchange(f"{self.field}={self.value}")
         yield ex
-        assert ex.line == "OK"
+        ex.check_ok()
 
 
 @dataclass
@@ -308,7 +302,7 @@ def execute(self) -> ExchangeGenerator[None]:
         # Multiline table with blank line to terminate
         ex = Exchange([f"{self.field}<<"] + self.value + [""])
         yield ex
-        assert ex.line == "OK"
+        ex.check_ok()
 
 
 class Arm(Command[None]):
@@ -317,7 +311,7 @@ class Arm(Command[None]):
     def execute(self) -> ExchangeGenerator[None]:
         ex = Exchange("*PCAP.ARM=")
         yield ex
-        assert ex.line == "OK"
+        ex.check_ok()
 
 
 class Disarm(Command[None]):
@@ -326,7 +320,7 @@ class Disarm(Command[None]):
     def execute(self) -> ExchangeGenerator[None]:
         ex = Exchange("*PCAP.DISARM=")
         yield ex
-        assert ex.line == "OK"
+        ex.check_ok()
 
 
 @dataclass

src/pandablocks/connections.py

@@ -120,16 +120,7 @@ class _ExchangeContext:
     generator: Optional[ExchangeGenerator[Any]] = None
 
     def exception(self, e: Exception) -> CommandException:
-        """Return a `CommandException` with the sent and received strings
-        in the text"""
-        msg = f"{self.command} ->"
-        if self.exchange.is_multiline:
-            for line in self.exchange.multiline:
-                msg += "\n    " + line
-        else:
-            msg += " " + self.exchange.line
-        if e.args:
-            msg += f"\n{type(e).__name__}:{e}"
+        msg = f"{self.command} raised error:\n{type(e).__name__}: {e}"
         return CommandException(msg).with_traceback(e.__traceback__)
 
 

tests/test_asyncio.py

@@ -27,7 +27,10 @@ async def test_asyncio_bad_put_raises(dummy_server_async):
     async with AsyncioClient("localhost") as client:
         with pytest.raises(CommandException) as cm:
             await asyncio.wait_for(client.send(Put("PCAP.thing", 1)), timeout=1)
-        assert str(cm.value) == "Put(field='PCAP.thing', value=1) -> ERR no such field"
+        assert (
+            str(cm.value) == "Put(field='PCAP.thing', value=1) raised error:\n"
+            "AssertionError: 'PCAP.thing=1' -> 'ERR no such field'"
+        )
     assert dummy_server_async.received == ["PCAP.thing=1"]
 
 

tests/test_blocking.py

@@ -17,7 +17,10 @@ def test_blocking_bad_put_raises(dummy_server_in_thread):
     with BlockingClient("localhost") as client:
         with pytest.raises(CommandException) as cm:
             client.send(Put("PCAP.thing", 1), timeout=1)
-        assert str(cm.value) == "Put(field='PCAP.thing', value=1) -> ERR no such field"
+        assert (
+            str(cm.value) == "Put(field='PCAP.thing', value=1) raised error:\n"
+            "AssertionError: 'PCAP.thing=1' -> 'ERR no such field'"
+        )
     assert dummy_server_in_thread.received == ["PCAP.thing=1"]
 
 

tests/test_pandablocks.py

@@ -83,7 +83,10 @@ def test_get_line_error_when_multiline():
     assert get_responses(conn, b"!ACTIVE 5 bit_out\n.\n") == [
         (
             cmd,
-            ACommandException("GetLine(field='PCAP.ACTIVE') ->\n    ACTIVE 5 bit_out"),
+            ACommandException(
+                "GetLine(field='PCAP.ACTIVE') raised error:\n"
+                "AssertionError: 'PCAP.ACTIVE?' -> '!ACTIVE 5 bit_out\\n.'"
+            ),
         )
     ]
 
@@ -95,7 +98,10 @@ def test_get_line_error_no_ok():
     assert get_responses(conn, b"NOT OK\n") == [
         (
             cmd,
-            ACommandException("GetLine(field='PCAP.ACTIVE') -> NOT OK"),
+            ACommandException(
+                "GetLine(field='PCAP.ACTIVE') raised error:\n"
+                "AssertionError: 'PCAP.ACTIVE?' -> 'NOT OK'"
+            ),
         )
     ]
 
@@ -117,7 +123,10 @@ def test_get_multiline_error_when_single_line():
     assert get_responses(conn, b"1\n") == [
         (
             cmd,
-            ACommandException("GetMultiline(field='PCAP.*') -> 1"),
+            ACommandException(
+                "GetMultiline(field='PCAP.*') raised error:\n"
+                "AssertionError: 'PCAP.*?' -> '1'"
+            ),
         )
     ]
 
@@ -143,7 +152,8 @@ def test_put_fails_with_single_line_exception():
         (
             cmd,
             ACommandException(
-                "Put(field='PCAP.blah', value='something') -> ERR No such field"
+                "Put(field='PCAP.blah', value='something') raised error:\n"
+                "AssertionError: 'PCAP.blah=something' -> 'ERR No such field'"
             ),
         )
     ]
@@ -157,11 +167,9 @@ def test_put_fails_with_multiline_exception():
         (
             cmd,
             ACommandException(
-                """\
-Put(field='PCAP.blah', value='something') ->
-    This is bad
-    Very bad
-    Don't do this"""
+                "Put(field='PCAP.blah', value='something') raised error:\n"
+                "AssertionError: 'PCAP.blah=something' -> "
+                '"!This is bad\\n!Very bad\\n!Don\'t do this\\n."'
             ),
         )
     ]
@@ -267,7 +275,8 @@ def test_get_block_info_error():
         (
             cmd,
             ACommandException(
-                "GetBlockInfo(skip_description=False) -> ERR Cannot read blocks"
+                "GetBlockInfo(skip_description=False) raised error:\n"
+                "AssertionError: '*BLOCKS?' -> 'ERR Cannot read blocks'"
             ),
         )
     ]
@@ -291,7 +300,8 @@ def test_get_block_info_desc_err():
         (
             cmd,
             ACommandException(
-                "GetBlockInfo(skip_description=False) -> ERR could not get description"
+                "GetBlockInfo(skip_description=False) raised error:\n"
+                "AssertionError: '*DESC.PCAP?' -> 'ERR could not get description'"
             ),
         )
     ]
@@ -416,7 +426,8 @@ def test_get_fields_non_existant_block():
         (
             cmd,
             ACommandException(
-                "GetFieldInfo(block='FOO', extended_metadata=True) -> ERR No such block"
+                "GetFieldInfo(block='FOO', extended_metadata=True) raised error:\n"
+                "AssertionError: 'FOO.*?' -> 'ERR No such block'"
             ),
         )
     ]