commit 1e1cb2e2eef4f39840b64a31c1a4d9d3b8a4a4cc
author Wyatt Hepler <hepler@google.com> Thu Sep 17 14:53:34 2020 -0700
committer CQ Bot Account <commit-bot@chromium.org> Thu Sep 17 23:34:48 2020 +0000
tree 2217e444f4d7ed951ed6bebbb6d8448b201f5343
parent b8db509530697c5036490a3d0ab1dad36e521f7b

pw_rpc: Fix Python client SERVER_ERROR handling

Call process_response for SERVER_ERRORs so that the implementation can
do cleanup for the RPC.

Change-Id: I8b1bc6ecb470426e8aa2986264c61c22487a6d72
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/18349
Reviewed-by: Keir Mierle <keir@google.com>
Commit-Queue: Wyatt Hepler <hepler@google.com>

pw_hdlc_lite/py/pw_hdlc_lite/rpc_console.py

diff --git a/pw_hdlc_lite/py/pw_hdlc_lite/rpc_console.py b/pw_hdlc_lite/py/pw_hdlc_lite/rpc_console.py
index 6bf40b0..443d50d 100644
--- a/pw_hdlc_lite/py/pw_hdlc_lite/rpc_console.py
+++ b/pw_hdlc_lite/py/pw_hdlc_lite/rpc_console.py
@@ -27,7 +27,7 @@
 
 An example echo RPC command:
 
-  cmd.rpcs.pw.rpc.EchoService.Echo(msg="hello!")
+  rpcs.pw.rpc.EchoService.Echo(msg="hello!")
 """
 
 import argparse
@@ -128,7 +128,8 @@
     _start_ipython_terminal(device, client)
 
 
-def main(device: str, baudrate: int, proto_globs: Collection[str]) -> int:
+def _prepare_console(device: str, baudrate: int,
+                     proto_globs: Collection[str]) -> int:
     if not proto_globs:
         proto_globs = ['**/*.proto']
 
@@ -147,5 +148,9 @@
     return 0
 
 
+def main() -> int:
+    return _prepare_console(**vars(_parse_args()))
+
+
 if __name__ == '__main__':
-    sys.exit(main(**vars(_parse_args())))
+    sys.exit(main())

pw_rpc/py/pw_rpc/client.py

diff --git a/pw_rpc/py/pw_rpc/client.py b/pw_rpc/py/pw_rpc/client.py
index a7c2920..755b521 100644
--- a/pw_rpc/py/pw_rpc/client.py
+++ b/pw_rpc/py/pw_rpc/client.py
@@ -86,16 +86,6 @@
 
         return True
 
-    def clear(self, rpc: PendingRpc) -> bool:
-        """Clears the RPC's pending status without sending a CANCEL packet."""
-        try:
-            _LOG.debug('Clearing %s', rpc)
-            del self._pending[rpc]
-        except KeyError:
-            return False
-
-        return True
-
     def get_pending(self, rpc: PendingRpc, status: Optional[Status]):
         if status is None:
             return self._pending[rpc][0]  # Unwrap the context from the list
@@ -349,13 +339,9 @@
 
         status = _decode_status(rpc, packet)
 
-        if packet.type == PacketType.SERVER_ERROR:
-            self._rpcs.clear(rpc)
-            _LOG.warning('%s: invocation failed with %s', rpc, status)
-            return Status.OK  # Handled packet, even though it was an error
-
         if packet.type not in (PacketType.RESPONSE,
-                               PacketType.SERVER_STREAM_END):
+                               PacketType.SERVER_STREAM_END,
+                               PacketType.SERVER_ERROR):
             _LOG.error('%s: unexpected PacketType %s', rpc, packet.type)
             _LOG.debug('Packet:\n%s', packet)
             return Status.OK
@@ -371,6 +357,12 @@
             _LOG.debug('Discarding response for %s, which is not pending', rpc)
             return Status.OK
 
+        if packet.type == PacketType.SERVER_ERROR:
+            _LOG.warning('%s: invocation failed with %s', rpc, status)
+
+            # Do not return yet -- call process_response so the ClientImpl can
+            # do any necessary cleanup.
+
         self._impl.process_response(self._rpcs,
                                     rpc,
                                     context,