session: forward error code to client

Since sysrepo 2.2.170, the return code of application callbacks is
ignored if they called sr_session_set_error_message() to report error
details. Instead, a generic SR_ERR_OPERATION_FAILED code is attached
with multiple error messages.

Since we have the possibility to do it, attach specific error codes
using sr_session_set_error() instead.

Adjust unit tests accordingly.

Link: sysrepo/sysrepo@282926f1d0d66
Signed-off-by: Robin Jarry <[email protected]>

Author: rjarry

cffi/cdefs.h

@@ -95,7 +95,7 @@ int sr_session_switch_ds(sr_session_ctx_t *, sr_datastore_t);
 sr_datastore_t sr_session_get_ds(sr_session_ctx_t *);
 sr_conn_ctx_t *sr_session_get_connection(sr_session_ctx_t *);
 int sr_session_get_error(sr_session_ctx_t *, const sr_error_info_t **);
-int sr_session_set_error_message(sr_session_ctx_t *, const char *, ...);
+int sr_session_set_error(sr_session_ctx_t *, const char *, sr_error_t, const char *, ...);
 const char *sr_session_get_orig_name(sr_session_ctx_t *session);
 int sr_session_set_orig_name(sr_session_ctx_t *session, const char *);
 int sr_session_get_orig_data(sr_session_ctx_t *session, uint32_t idx, uint32_t *size, const void **data);

sysrepo/session.py

@@ -11,6 +11,7 @@
 from _sysrepo import ffi, lib
 from .change import Change
 from .errors import (
+    SysrepoError,
     SysrepoInternalError,
     SysrepoNotFoundError,
     SysrepoUnsupportedError,
@@ -164,7 +165,7 @@ def switch_datastore(self, datastore: str) -> None:
         ds = datastore_value(datastore)
         check_call(lib.sr_session_switch_ds, self.cdata, ds)
 
-    def set_error(self, message: str):
+    def set_error(self, err: BaseException):
         """
         Set detailed error information into provided session. Used to notify the client
         library about errors that occurred in the application code. Does not print the
@@ -173,13 +174,24 @@ def set_error(self, message: str):
         Intended for change, RPC/action, or operational callbacks to be used on the
         provided session.
 
-        :arg str message:
-            The detailed error message.
+        :arg BaseException err:
+            The python exception object that carries the error information.
         """
         if not self.is_implicit:
             raise SysrepoUnsupportedError("can only report errors on implicit sessions")
+        if isinstance(err, SysrepoError):
+            code = err.rc
+            message = err.msg
+        else:
+            code = lib.SR_ERR_CALLBACK_FAILED
+            message = str(err)
         check_call(
-            lib.sr_session_set_error_message, self.cdata, str2c("%s"), str2c(message)
+            lib.sr_session_set_error,
+            self.cdata,
+            ffi.NULL,
+            code,
+            str2c("%s"),
+            str2c(message),
         )
 
     def get_originator_name(self) -> str:

sysrepo/subscription.py

@@ -304,7 +304,7 @@ def module_change_callback(session, sub_id, module, xpath, event, req_id, priv):
             and isinstance(session, SysrepoSession)
             and isinstance(xpath, str)
         ):
-            session.set_error(e.msg)
+            session.set_error(e)
         return e.rc
 
     except BaseException as e:
@@ -317,7 +317,7 @@ def module_change_callback(session, sub_id, module, xpath, event, req_id, priv):
             and isinstance(session, SysrepoSession)
             and isinstance(xpath, str)
         ):
-            session.set_error(str(e))
+            session.set_error(e)
         return lib.SR_ERR_CALLBACK_FAILED
 
 
@@ -421,7 +421,7 @@ def oper_data_callback(session, sub_id, module, xpath, req_xpath, req_id, parent
 
     except SysrepoError as e:
         if e.msg and isinstance(session, SysrepoSession) and isinstance(xpath, str):
-            session.set_error(e.msg)
+            session.set_error(e)
         return e.rc
 
     except BaseException as e:
@@ -430,7 +430,7 @@ def oper_data_callback(session, sub_id, module, xpath, req_xpath, req_id, parent
         # We are in a C callback, we cannot let any error pass
         LOG.exception("%r callback failed", locals().get("callback", priv))
         if isinstance(session, SysrepoSession) and isinstance(xpath, str):
-            session.set_error(str(e))
+            session.set_error(e)
         return lib.SR_ERR_CALLBACK_FAILED
 
 
@@ -548,7 +548,7 @@ def rpc_callback(session, sub_id, xpath, input_node, event, req_id, output_node,
 
     except SysrepoError as e:
         if e.msg and isinstance(session, SysrepoSession) and isinstance(xpath, str):
-            session.set_error(e.msg)
+            session.set_error(e)
         return e.rc
 
     except BaseException as e:
@@ -557,7 +557,7 @@ def rpc_callback(session, sub_id, xpath, input_node, event, req_id, output_node,
         # We are in a C callback, we cannot let any error pass
         LOG.exception("%r callback failed", locals().get("callback", priv))
         if isinstance(session, SysrepoSession) and isinstance(xpath, str):
-            session.set_error(str(e))
+            session.set_error(e)
         return lib.SR_ERR_CALLBACK_FAILED
 
 

tests/test_subs_module_change.py

@@ -74,7 +74,7 @@ def module_change_cb(event, req_id, changes, private_data):
             # 2.
             sent_config = {"conf": {"system": {"hostname": "INVALID"}}}
             expected_changes = []
-            with self.assertRaises(sysrepo.SysrepoCallbackFailedError):
+            with self.assertRaises(sysrepo.SysrepoValidationFailedError):
                 ch_sess.replace_config(sent_config, "sysrepo-example", strict=True)
             # 3.
             sent_config = {
@@ -347,7 +347,7 @@ def module_change_cb(session, event, req_id, private_data):
             # 2.
             sent_config = {"conf": {"system": {"hostname": "INVALID"}}}
             expected_changes = []
-            with self.assertRaises(sysrepo.SysrepoCallbackFailedError):
+            with self.assertRaises(sysrepo.SysrepoValidationFailedError):
                 ch_sess.replace_config(sent_config, "sysrepo-example", strict=True)
             # 3.
             sent_config = {