From 61551b07d6145ad3dd87434616241da129b69538 Mon Sep 17 00:00:00 2001
From: Olof hagsand <olof@hagsand.se>
Date: Tue, 14 Nov 2017 23:14:51 +0100
Subject: [PATCH] When user callbacks p_statedata() clixon_backend nolonger
 silently exit

---
 CHANGELOG.md                  |  7 +++---
 apps/backend/backend_client.c | 33 +++++++++++++++++++--------
 apps/backend/backend_plugin.c | 43 ++++++++++++++++++-----------------
 3 files changed, 49 insertions(+), 34 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 42152378..7066a569 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,7 +1,8 @@
 # Clixon CHANGELOG
-
-* When user callbacks such as statedata() call returns -1, clixon_backend no
-  longer silently exits. Instead a log is printed and an RPC error is returned.
+	
+* When user callbacks p_statedata() or rpc callback call returns -1,
+  clixon_backend nolonger silently exits. Instead a log is printed and an
+  RPC error is returned.
 
 * Support for non-line scrolling in CLI, eg wrap lines. Set with:
   CLICON_CLI_LINESCROLLING 0
diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c
index 9f0c805e..5eccb6df 100644
--- a/apps/backend/backend_client.c
+++ b/apps/backend/backend_client.c
@@ -275,6 +275,7 @@ from_client_get(clicon_handle h,
     cxobj *xfilter;
     char  *selector = "/";
     cxobj *xret = NULL;
+    int    ret;
     
     if ((xfilter = xml_find(xe, "filter")) != NULL)
 	if ((selector = xml_find_value(xfilter, "select"))==NULL)
@@ -291,18 +292,30 @@ from_client_get(clicon_handle h,
     }
     /* Get state data from plugins as defined by plugin_statedata(), if any */
     assert(xret);
-    if (backend_statedata_call(h, selector, xret) < 0)
+    clicon_err_reset();
+    if ((ret = backend_statedata_call(h, selector, xret)) < 0)
 	goto done;
-    cprintf(cbret, "<rpc-reply>");
-    if (xret==NULL)
-	cprintf(cbret, "<data/>");
-    else{
-	if (xml_name_set(xret, "data") < 0)
-	    goto done;
-	if (clicon_xml2cbuf(cbret, xret, 0, 0) < 0)
-	    goto done;
+    if (ret == 0){ /* OK */
+	cprintf(cbret, "<rpc-reply>");
+	if (xret==NULL)
+	    cprintf(cbret, "<data/>");
+	else{
+	    if (xml_name_set(xret, "data") < 0)
+		goto done;
+	    if (clicon_xml2cbuf(cbret, xret, 0, 0) < 0)
+		goto done;
+	}
+	cprintf(cbret, "</rpc-reply>");
+    }
+    else { /* 1 Error from callback */
+	cprintf(cbret, "<rpc-reply><rpc-error>"
+		"<error-tag>operation-failed</error-tag>"
+		"<error-type>rpc</error-type>"
+		"<error-severity>error</error-severity>"
+		"<error-message>Internal error:%s</error-message>"
+		"</rpc-error></rpc-reply>", clicon_err_reason);
+	clicon_log(LOG_NOTICE, "%s Error in backend_statedata_call:%s", __FUNCTION__, xml_name(xe));
     }
-    cprintf(cbret, "</rpc-reply>");
  ok:
     retval = 0;
  done:
diff --git a/apps/backend/backend_plugin.c b/apps/backend/backend_plugin.c
index 9bb6b131..cdc4b958 100644
--- a/apps/backend/backend_plugin.c
+++ b/apps/backend/backend_plugin.c
@@ -728,6 +728,7 @@ plugin_transaction_abort(clicon_handle       h,
  * @param[in,out] xml  XML tree.
  * @retval -1   Error
  * @retval  0   OK
+ * @retval  1   Statedata callback failed
  */
 int
 backend_statedata_call(clicon_handle        h,
@@ -755,8 +756,10 @@ backend_statedata_call(clicon_handle        h,
 	if (p->p_statedata) {
 	    if ((x = xml_new("config", NULL)) == NULL)
 		goto done;
-	    if ((p->p_statedata)(h, xpath, x) < 0)
-		goto done;
+	    if ((p->p_statedata)(h, xpath, x) < 0){
+		retval = 1;
+		goto done; /* Dont quit here on user callbacks */
+	    }
 	    if (xml_merge(xtop, x, yspec) < 0)
 		goto done;
 	    if (x){
@@ -765,27 +768,25 @@ backend_statedata_call(clicon_handle        h,
 	    }
 	}
     }
-    {
-	/* Code complex to filter out anything that is outside of xpath */
-	if (xpath_vec(xtop, xpath?xpath:"/", &xvec, &xlen) < 0)
-	    goto done;
+    /* Code complex to filter out anything that is outside of xpath */
+    if (xpath_vec(xtop, xpath?xpath:"/", &xvec, &xlen) < 0)
+	goto done;
 
-	/* If vectors are specified then mark the nodes found and
-	 * then filter out everything else,
-	 * otherwise return complete tree.
-	 */
-	if (xvec != NULL){
-	    for (i=0; i<xlen; i++)
-		xml_flag_set(xvec[i], XML_FLAG_MARK);
-	}
-	/* Remove everything that is not marked */
-	if (!xml_flag(xtop, XML_FLAG_MARK))
-	    if (xml_tree_prune_flagged_sub(xtop, XML_FLAG_MARK, 1, NULL) < 0)
-		goto done;
-	/* reset flag */
-	if (xml_apply(xtop, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)XML_FLAG_MARK) < 0)
-	    goto done;
+    /* If vectors are specified then mark the nodes found and
+     * then filter out everything else,
+     * otherwise return complete tree.
+     */
+    if (xvec != NULL){
+	for (i=0; i<xlen; i++)
+	    xml_flag_set(xvec[i], XML_FLAG_MARK);
     }
+    /* Remove everything that is not marked */
+    if (!xml_flag(xtop, XML_FLAG_MARK))
+	if (xml_tree_prune_flagged_sub(xtop, XML_FLAG_MARK, 1, NULL) < 0)
+	    goto done;
+    /* reset flag */
+    if (xml_apply(xtop, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)XML_FLAG_MARK) < 0)
+	goto done;
     retval = 0;
  done:
     if (x)