lib: return human-readable error messages to the northbound clients

Instead of returning only error codes (e.g. NB_ERR_VALIDATION)
to the northbound clients, do better than that and also return
a human-readable error message. This should make FRR more
automation-friendly since operators won't need to dig into system
logs to find out what went wrong in the case of an error.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

10 files changed, 323 insertions, 168 deletions

diff --git a/lib/libfrr.c b/lib/libfrr.c
index d52cef3fe9..cac9929577 100644
--- a/lib/libfrr.c
+++ b/lib/libfrr.c
@@ -903,15 +903,17 @@ static int frr_config_read_in(struct thread *t)
 	 */
 	if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL) {
 		struct nb_context context = {};
+		char errmsg[BUFSIZ] = {0};
 		int ret;
 
 		context.client = NB_CLIENT_CLI;
 		ret = nb_candidate_commit(&context, vty_shared_candidate_config,
-					  true, "Read configuration file",
-					  NULL);
+					  true, "Read configuration file", NULL,
+					  errmsg, sizeof(errmsg));
 		if (ret != NB_OK && ret != NB_ERR_NO_CHANGES)
-			zlog_err("%s: failed to read configuration file.",
-				 __func__);
+			zlog_err(
+				"%s: failed to read configuration file: %s (%s)",
+				__func__, nb_err_name(ret), errmsg);
 	}
 
 	return 0;
diff --git a/lib/northbound.c b/lib/northbound.c
index e5349ad4eb..14b31ef157 100644
--- a/lib/northbound.c
+++ b/lib/northbound.c
@@ -64,18 +64,22 @@ static bool transaction_in_progress;
 
 static int nb_callback_pre_validate(struct nb_context *context,
 				    const struct nb_node *nb_node,
-				    const struct lyd_node *dnode);
+				    const struct lyd_node *dnode, char *errmsg,
+				    size_t errmsg_len);
 static int nb_callback_configuration(struct nb_context *context,
 				     const enum nb_event event,
-				     struct nb_config_change *change);
-static struct nb_transaction *nb_transaction_new(struct nb_context *context,
-						 struct nb_config *config,
-						 struct nb_config_cbs *changes,
-						 const char *comment);
+				     struct nb_config_change *change,
+				     char *errmsg, size_t errmsg_len);
+static struct nb_transaction *
+nb_transaction_new(struct nb_context *context, struct nb_config *config,
+		   struct nb_config_cbs *changes, const char *comment,
+		   char *errmsg, size_t errmsg_len);
 static void nb_transaction_free(struct nb_transaction *transaction);
 static int nb_transaction_process(enum nb_event event,
-				  struct nb_transaction *transaction);
-static void nb_transaction_apply_finish(struct nb_transaction *transaction);
+				  struct nb_transaction *transaction,
+				  char *errmsg, size_t errmsg_len);
+static void nb_transaction_apply_finish(struct nb_transaction *transaction,
+					char *errmsg, size_t errmsg_len);
 static int nb_oper_data_iter_node(const struct lys_node *snode,
 				  const char *xpath, const void *list_entry,
 				  const struct yang_list_keys *list_keys,
@@ -581,13 +585,16 @@ int nb_candidate_update(struct nb_config *candidate)
  * WARNING: lyd_validate() can change the configuration as part of the
  * validation process.
  */
-static int nb_candidate_validate_yang(struct nb_config *candidate)
+static int nb_candidate_validate_yang(struct nb_config *candidate, char *errmsg,
+				      size_t errmsg_len)
 {
 	if (lyd_validate(&candidate->dnode,
 			 LYD_OPT_STRICT | LYD_OPT_CONFIG | LYD_OPT_WHENAUTODEL,
 			 ly_native_ctx)
-	    != 0)
+	    != 0) {
+		yang_print_errors(ly_native_ctx, errmsg, errmsg_len);
 		return NB_ERR_VALIDATION;
+	}
 
 	return NB_OK;
 }
@@ -595,7 +602,8 @@ static int nb_candidate_validate_yang(struct nb_config *candidate)
 /* Perform code-level validation using the northbound callbacks. */
 static int nb_candidate_validate_code(struct nb_context *context,
 				      struct nb_config *candidate,
-				      struct nb_config_cbs *changes)
+				      struct nb_config_cbs *changes,
+				      char *errmsg, size_t errmsg_len)
 {
 	struct nb_config_cb *cb;
 	struct lyd_node *root, *next, *child;
@@ -610,7 +618,8 @@ static int nb_candidate_validate_code(struct nb_context *context,
 			if (!nb_node->cbs.pre_validate)
 				goto next;
 
-			ret = nb_callback_pre_validate(context, nb_node, child);
+			ret = nb_callback_pre_validate(context, nb_node, child,
+						       errmsg, errmsg_len);
 			if (ret != NB_OK)
 				return NB_ERR_VALIDATION;
 
@@ -623,8 +632,8 @@ static int nb_candidate_validate_code(struct nb_context *context,
 	RB_FOREACH (cb, nb_config_cbs, changes) {
 		struct nb_config_change *change = (struct nb_config_change *)cb;
 
-		ret = nb_callback_configuration(context, NB_EV_VALIDATE,
-						change);
+		ret = nb_callback_configuration(context, NB_EV_VALIDATE, change,
+						errmsg, errmsg_len);
 		if (ret != NB_OK)
 			return NB_ERR_VALIDATION;
 	}
@@ -633,17 +642,20 @@ static int nb_candidate_validate_code(struct nb_context *context,
 }
 
 int nb_candidate_validate(struct nb_context *context,
-			  struct nb_config *candidate)
+			  struct nb_config *candidate, char *errmsg,
+			  size_t errmsg_len)
 {
 	struct nb_config_cbs changes;
 	int ret;
 
-	if (nb_candidate_validate_yang(candidate) != NB_OK)
+	if (nb_candidate_validate_yang(candidate, errmsg, sizeof(errmsg_len))
+	    != NB_OK)
 		return NB_ERR_VALIDATION;
 
 	RB_INIT(nb_config_cbs, &changes);
 	nb_config_diff(running_config, candidate, &changes);
-	ret = nb_candidate_validate_code(context, candidate, &changes);
+	ret = nb_candidate_validate_code(context, candidate, &changes, errmsg,
+					 errmsg_len);
 	nb_config_diff_del_changes(&changes);
 
 	return ret;
@@ -652,11 +664,13 @@ int nb_candidate_validate(struct nb_context *context,
 int nb_candidate_commit_prepare(struct nb_context *context,
 				struct nb_config *candidate,
 				const char *comment,
-				struct nb_transaction **transaction)
+				struct nb_transaction **transaction,
+				char *errmsg, size_t errmsg_len)
 {
 	struct nb_config_cbs changes;
 
-	if (nb_candidate_validate_yang(candidate) != NB_OK) {
+	if (nb_candidate_validate_yang(candidate, errmsg, errmsg_len)
+	    != NB_OK) {
 		flog_warn(EC_LIB_NB_CANDIDATE_INVALID,
 			  "%s: failed to validate candidate configuration",
 			  __func__);
@@ -668,7 +682,9 @@ int nb_candidate_commit_prepare(struct nb_context *context,
 	if (RB_EMPTY(nb_config_cbs, &changes))
 		return NB_ERR_NO_CHANGES;
 
-	if (nb_candidate_validate_code(context, candidate, &changes) != NB_OK) {
+	if (nb_candidate_validate_code(context, candidate, &changes, errmsg,
+				       errmsg_len)
+	    != NB_OK) {
 		flog_warn(EC_LIB_NB_CANDIDATE_INVALID,
 			  "%s: failed to validate candidate configuration",
 			  __func__);
@@ -676,29 +692,37 @@ int nb_candidate_commit_prepare(struct nb_context *context,
 		return NB_ERR_VALIDATION;
 	}
 
-	*transaction =
-		nb_transaction_new(context, candidate, &changes, comment);
+	*transaction = nb_transaction_new(context, candidate, &changes, comment,
+					  errmsg, errmsg_len);
 	if (*transaction == NULL) {
 		flog_warn(EC_LIB_NB_TRANSACTION_CREATION_FAILED,
-			  "%s: failed to create transaction", __func__);
+			  "%s: failed to create transaction: %s", __func__,
+			  errmsg);
 		nb_config_diff_del_changes(&changes);
 		return NB_ERR_LOCKED;
 	}
 
-	return nb_transaction_process(NB_EV_PREPARE, *transaction);
+	return nb_transaction_process(NB_EV_PREPARE, *transaction, errmsg,
+				      errmsg_len);
 }
 
 void nb_candidate_commit_abort(struct nb_transaction *transaction)
 {
-	(void)nb_transaction_process(NB_EV_ABORT, transaction);
+	char errmsg[BUFSIZ] = {0};
+
+	(void)nb_transaction_process(NB_EV_ABORT, transaction, errmsg,
+				     sizeof(errmsg));
 	nb_transaction_free(transaction);
 }
 
 void nb_candidate_commit_apply(struct nb_transaction *transaction,
 			       bool save_transaction, uint32_t *transaction_id)
 {
-	(void)nb_transaction_process(NB_EV_APPLY, transaction);
-	nb_transaction_apply_finish(transaction);
+	char errmsg[BUFSIZ] = {0};
+
+	(void)nb_transaction_process(NB_EV_APPLY, transaction, errmsg,
+				     sizeof(errmsg));
+	nb_transaction_apply_finish(transaction, errmsg, sizeof(errmsg));
 
 	/* Replace running by candidate. */
 	transaction->config->version++;
@@ -715,13 +739,14 @@ void nb_candidate_commit_apply(struct nb_transaction *transaction,
 
 int nb_candidate_commit(struct nb_context *context, struct nb_config *candidate,
 			bool save_transaction, const char *comment,
-			uint32_t *transaction_id)
+			uint32_t *transaction_id, char *errmsg,
+			size_t errmsg_len)
 {
 	struct nb_transaction *transaction = NULL;
 	int ret;
 
 	ret = nb_candidate_commit_prepare(context, candidate, comment,
-					  &transaction);
+					  &transaction, errmsg, errmsg_len);
 	/*
 	 * Apply the changes if the preparation phase succeeded. Otherwise abort
 	 * the transaction.
@@ -808,7 +833,8 @@ static void nb_log_config_callback(const enum nb_event event,
 static int nb_callback_create(struct nb_context *context,
 			      const struct nb_node *nb_node,
 			      enum nb_event event, const struct lyd_node *dnode,
-			      union nb_resource *resource)
+			      union nb_resource *resource, char *errmsg,
+			      size_t errmsg_len)
 {
 	struct nb_cb_create_args args = {};
 
@@ -818,13 +844,16 @@ static int nb_callback_create(struct nb_context *context,
 	args.event = event;
 	args.dnode = dnode;
 	args.resource = resource;
+	args.errmsg = errmsg;
+	args.errmsg_len = errmsg_len;
 	return nb_node->cbs.create(&args);
 }
 
 static int nb_callback_modify(struct nb_context *context,
 			      const struct nb_node *nb_node,
 			      enum nb_event event, const struct lyd_node *dnode,
-			      union nb_resource *resource)
+			      union nb_resource *resource, char *errmsg,
+			      size_t errmsg_len)
 {
 	struct nb_cb_modify_args args = {};
 
@@ -834,13 +863,16 @@ static int nb_callback_modify(struct nb_context *context,
 	args.event = event;
 	args.dnode = dnode;
 	args.resource = resource;
+	args.errmsg = errmsg;
+	args.errmsg_len = errmsg_len;
 	return nb_node->cbs.modify(&args);
 }
 
 static int nb_callback_destroy(struct nb_context *context,
 			       const struct nb_node *nb_node,
 			       enum nb_event event,
-			       const struct lyd_node *dnode)
+			       const struct lyd_node *dnode, char *errmsg,
+			       size_t errmsg_len)
 {
 	struct nb_cb_destroy_args args = {};
 
@@ -849,12 +881,15 @@ static int nb_callback_destroy(struct nb_context *context,
 	args.context = context;
 	args.event = event;
 	args.dnode = dnode;
+	args.errmsg = errmsg;
+	args.errmsg_len = errmsg_len;
 	return nb_node->cbs.destroy(&args);
 }
 
 static int nb_callback_move(struct nb_context *context,
 			    const struct nb_node *nb_node, enum nb_event event,
-			    const struct lyd_node *dnode)
+			    const struct lyd_node *dnode, char *errmsg,
+			    size_t errmsg_len)
 {
 	struct nb_cb_move_args args = {};
 
@@ -863,24 +898,30 @@ static int nb_callback_move(struct nb_context *context,
 	args.context = context;
 	args.event = event;
 	args.dnode = dnode;
+	args.errmsg = errmsg;
+	args.errmsg_len = errmsg_len;
 	return nb_node->cbs.move(&args);
 }
 
 static int nb_callback_pre_validate(struct nb_context *context,
 				    const struct nb_node *nb_node,
-				    const struct lyd_node *dnode)
+				    const struct lyd_node *dnode, char *errmsg,
+				    size_t errmsg_len)
 {
 	struct nb_cb_pre_validate_args args = {};
 
 	nb_log_config_callback(NB_EV_VALIDATE, NB_OP_PRE_VALIDATE, dnode);
 
 	args.dnode = dnode;
+	args.errmsg = errmsg;
+	args.errmsg_len = errmsg_len;
 	return nb_node->cbs.pre_validate(&args);
 }
 
 static void nb_callback_apply_finish(struct nb_context *context,
 				     const struct nb_node *nb_node,
-				     const struct lyd_node *dnode)
+				     const struct lyd_node *dnode, char *errmsg,
+				     size_t errmsg_len)
 {
 	struct nb_cb_apply_finish_args args = {};
 
@@ -888,6 +929,8 @@ static void nb_callback_apply_finish(struct nb_context *context,
 
 	args.context = context;
 	args.dnode = dnode;
+	args.errmsg = errmsg;
+	args.errmsg_len = errmsg_len;
 	nb_node->cbs.apply_finish(&args);
 }
 
@@ -969,7 +1012,8 @@ int nb_callback_rpc(const struct nb_node *nb_node, const char *xpath,
  */
 static int nb_callback_configuration(struct nb_context *context,
 				     const enum nb_event event,
-				     struct nb_config_change *change)
+				     struct nb_config_change *change,
+				     char *errmsg, size_t errmsg_len)
 {
 	enum nb_operation operation = change->cb.operation;
 	char xpath[XPATH_MAXLEN];
@@ -986,17 +1030,19 @@ static int nb_callback_configuration(struct nb_context *context,
 	switch (operation) {
 	case NB_OP_CREATE:
 		ret = nb_callback_create(context, nb_node, event, dnode,
-					 resource);
+					 resource, errmsg, errmsg_len);
 		break;
 	case NB_OP_MODIFY:
 		ret = nb_callback_modify(context, nb_node, event, dnode,
-					 resource);
+					 resource, errmsg, errmsg_len);
 		break;
 	case NB_OP_DESTROY:
-		ret = nb_callback_destroy(context, nb_node, event, dnode);
+		ret = nb_callback_destroy(context, nb_node, event, dnode,
+					  errmsg, errmsg_len);
 		break;
 	case NB_OP_MOVE:
-		ret = nb_callback_move(context, nb_node, event, dnode);
+		ret = nb_callback_move(context, nb_node, event, dnode, errmsg,
+				       errmsg_len);
 		break;
 	default:
 		yang_dnode_get_path(dnode, xpath, sizeof(xpath));
@@ -1037,34 +1083,36 @@ static int nb_callback_configuration(struct nb_context *context,
 		}
 
 		flog(priority, ref,
-		     "%s: error processing configuration change: error [%s] event [%s] operation [%s] xpath [%s]",
-		     __func__, nb_err_name(ret), nb_event_name(event),
+		     "error processing configuration change: error [%s] event [%s] operation [%s] xpath [%s]",
+		     nb_err_name(ret), nb_event_name(event),
 		     nb_operation_name(operation), xpath);
+		if (strlen(errmsg) > 0)
+			flog(priority, ref,
+			     "error processing configuration change: %s",
+			     errmsg);
 	}
 
 	return ret;
 }
 
-static struct nb_transaction *nb_transaction_new(struct nb_context *context,
-						 struct nb_config *config,
-						 struct nb_config_cbs *changes,
-						 const char *comment)
+static struct nb_transaction *
+nb_transaction_new(struct nb_context *context, struct nb_config *config,
+		   struct nb_config_cbs *changes, const char *comment,
+		   char *errmsg, size_t errmsg_len)
 {
 	struct nb_transaction *transaction;
 
 	if (nb_running_lock_check(context->client, context->user)) {
-		flog_warn(
-			EC_LIB_NB_TRANSACTION_CREATION_FAILED,
-			"%s: running configuration is locked by another client",
-			__func__);
+		strlcpy(errmsg,
+			"running configuration is locked by another client",
+			errmsg_len);
 		return NULL;
 	}
 
 	if (transaction_in_progress) {
-		flog_warn(
-			EC_LIB_NB_TRANSACTION_CREATION_FAILED,
-			"%s: error - there's already another transaction in progress",
-			__func__);
+		strlcpy(errmsg,
+			"there's already another transaction in progress",
+			errmsg_len);
 		return NULL;
 	}
 	transaction_in_progress = true;
@@ -1089,7 +1137,8 @@ static void nb_transaction_free(struct nb_transaction *transaction)
 
 /* Process all configuration changes associated to a transaction. */
 static int nb_transaction_process(enum nb_event event,
-				  struct nb_transaction *transaction)
+				  struct nb_transaction *transaction,
+				  char *errmsg, size_t errmsg_len)
 {
 	struct nb_config_cb *cb;
 
@@ -1106,7 +1155,7 @@ static int nb_transaction_process(enum nb_event event,
 
 		/* Call the appropriate callback. */
 		ret = nb_callback_configuration(transaction->context, event,
-						change);
+						change, errmsg, errmsg_len);
 		switch (event) {
 		case NB_EV_PREPARE:
 			if (ret != NB_OK)
@@ -1160,7 +1209,8 @@ nb_apply_finish_cb_find(struct nb_config_cbs *cbs,
 }
 
 /* Call the 'apply_finish' callbacks. */
-static void nb_transaction_apply_finish(struct nb_transaction *transaction)
+static void nb_transaction_apply_finish(struct nb_transaction *transaction,
+					char *errmsg, size_t errmsg_len)
 {
 	struct nb_config_cbs cbs;
 	struct nb_config_cb *cb;
@@ -1220,7 +1270,7 @@ static void nb_transaction_apply_finish(struct nb_transaction *transaction)
 	/* Call the 'apply_finish' callbacks, sorted by their priorities. */
 	RB_FOREACH (cb, nb_config_cbs, &cbs)
 		nb_callback_apply_finish(transaction->context, cb->nb_node,
-					 cb->dnode);
+					 cb->dnode, errmsg, errmsg_len);
 
 	/* Release memory. */
 	while (!RB_EMPTY(nb_config_cbs, &cbs)) {
@@ -1935,7 +1985,7 @@ const char *nb_err_name(enum nb_error error)
 	case NB_ERR_LOCKED:
 		return "resource is locked";
 	case NB_ERR_VALIDATION:
-		return "validation error";
+		return "validation";
 	case NB_ERR_RESOURCE:
 		return "failed to allocate resource";
 	case NB_ERR_INCONSISTENCY:
diff --git a/lib/northbound.h b/lib/northbound.h
index caa7a9f82d..40a97dad6b 100644
--- a/lib/northbound.h
+++ b/lib/northbound.h
@@ -110,6 +110,12 @@ struct nb_cb_create_args {
 	 * resource(s). It's set to NULL when the event is NB_EV_VALIDATE.
 	 */
 	union nb_resource *resource;
+
+	/* Buffer to store human-readable error message in case of error. */
+	char *errmsg;
+
+	/* Size of errmsg. */
+	size_t errmsg_len;
 };
 
 struct nb_cb_modify_args {
@@ -132,6 +138,12 @@ struct nb_cb_modify_args {
 	 * resource(s). It's set to NULL when the event is NB_EV_VALIDATE.
 	 */
 	union nb_resource *resource;
+
+	/* Buffer to store human-readable error message in case of error. */
+	char *errmsg;
+
+	/* Size of errmsg. */
+	size_t errmsg_len;
 };
 
 struct nb_cb_destroy_args {
@@ -146,6 +158,12 @@ struct nb_cb_destroy_args {
 
 	/* libyang data node that is being deleted. */
 	const struct lyd_node *dnode;
+
+	/* Buffer to store human-readable error message in case of error. */
+	char *errmsg;
+
+	/* Size of errmsg. */
+	size_t errmsg_len;
 };
 
 struct nb_cb_move_args {
@@ -160,6 +178,12 @@ struct nb_cb_move_args {
 
 	/* libyang data node that is being moved. */
 	const struct lyd_node *dnode;
+
+	/* Buffer to store human-readable error message in case of error. */
+	char *errmsg;
+
+	/* Size of errmsg. */
+	size_t errmsg_len;
 };
 
 struct nb_cb_pre_validate_args {
@@ -168,6 +192,12 @@ struct nb_cb_pre_validate_args {
 
 	/* libyang data node associated with the 'pre_validate' callback. */
 	const struct lyd_node *dnode;
+
+	/* Buffer to store human-readable error message in case of error. */
+	char *errmsg;
+
+	/* Size of errmsg. */
+	size_t errmsg_len;
 };
 
 struct nb_cb_apply_finish_args {
@@ -176,6 +206,12 @@ struct nb_cb_apply_finish_args {
 
 	/* libyang data node associated with the 'apply_finish' callback. */
 	const struct lyd_node *dnode;
+
+	/* Buffer to store human-readable error message in case of error. */
+	char *errmsg;
+
+	/* Size of errmsg. */
+	size_t errmsg_len;
 };
 
 struct nb_cb_get_elem_args {
@@ -809,11 +845,18 @@ extern int nb_candidate_update(struct nb_config *candidate);
  * candidate
  *    Candidate configuration to validate.
  *
+ * errmsg
+ *    Buffer to store human-readable error message in case of error.
+ *
+ * errmsg_len
+ *    Size of errmsg.
+ *
  * Returns:
  *    NB_OK on success, NB_ERR_VALIDATION otherwise.
  */
 extern int nb_candidate_validate(struct nb_context *context,
-				 struct nb_config *candidate);
+				 struct nb_config *candidate, char *errmsg,
+				 size_t errmsg_len);
 
 /*
  * Create a new configuration transaction but do not commit it yet. Only
@@ -835,6 +878,12 @@ extern int nb_candidate_validate(struct nb_context *context,
  *    nb_candidate_commit_abort() or committed using
  *    nb_candidate_commit_apply().
  *
+ * errmsg
+ *    Buffer to store human-readable error message in case of error.
+ *
+ * errmsg_len
+ *    Size of errmsg.
+ *
  * Returns:
  *    - NB_OK on success.
  *    - NB_ERR_NO_CHANGES when the candidate is identical to the running
@@ -848,7 +897,8 @@ extern int nb_candidate_validate(struct nb_context *context,
 extern int nb_candidate_commit_prepare(struct nb_context *context,
 				       struct nb_config *candidate,
 				       const char *comment,
-				       struct nb_transaction **transaction);
+				       struct nb_transaction **transaction,
+				       char *errmsg, size_t errmsg_len);
 
 /*
  * Abort a previously created configuration transaction, releasing all resources
@@ -901,6 +951,12 @@ extern void nb_candidate_commit_apply(struct nb_transaction *transaction,
  * transaction_id
  *    Optional output parameter providing the ID of the committed transaction.
  *
+ * errmsg
+ *    Buffer to store human-readable error message in case of error.
+ *
+ * errmsg_len
+ *    Size of errmsg.
+ *
  * Returns:
  *    - NB_OK on success.
  *    - NB_ERR_NO_CHANGES when the candidate is identical to the running
@@ -914,7 +970,8 @@ extern void nb_candidate_commit_apply(struct nb_transaction *transaction,
 extern int nb_candidate_commit(struct nb_context *context,
 			       struct nb_config *candidate,
 			       bool save_transaction, const char *comment,
-			       uint32_t *transaction_id);
+			       uint32_t *transaction_id, char *errmsg,
+			       size_t errmsg_len);
 
 /*
  * Lock the running configuration.
diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c
index 544b2434eb..105fc83cef 100644
--- a/lib/northbound_cli.c
+++ b/lib/northbound_cli.c
@@ -46,23 +46,11 @@ struct debug nb_dbg_libyang = {0, "libyang debugging"};
 struct nb_config *vty_shared_candidate_config;
 static struct thread_master *master;
 
-static void vty_show_libyang_errors(struct vty *vty, struct ly_ctx *ly_ctx)
+static void vty_show_nb_errors(struct vty *vty, int error, const char *errmsg)
 {
-	struct ly_err_item *ei;
-	const char *path;
-
-	ei = ly_err_first(ly_ctx);
-	if (!ei)
-		return;
-
-	for (; ei; ei = ei->next)
-		vty_out(vty, "%s\n", ei->msg);
-
-	path = ly_errpath(ly_ctx);
-	if (path)
-		vty_out(vty, "YANG path: %s\n", path);
-
-	ly_err_clean(ly_ctx, NULL);
+	vty_out(vty, "Error type: %s\n", nb_err_name(error));
+	if (strlen(errmsg) > 0)
+		vty_out(vty, "Error description: %s\n", errmsg);
 }
 
 void nb_cli_enqueue_change(struct vty *vty, const char *xpath,
@@ -158,28 +146,31 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...)
 	}
 
 	if (error) {
+		char buf[BUFSIZ];
+
 		/*
 		 * Failure to edit the candidate configuration should never
 		 * happen in practice, unless there's a bug in the code. When
 		 * that happens, log the error but otherwise ignore it.
 		 */
 		vty_out(vty, "%% Failed to edit configuration.\n\n");
-		vty_show_libyang_errors(vty, ly_native_ctx);
+		vty_out(vty, "%s",
+			yang_print_errors(ly_native_ctx, buf, sizeof(buf)));
 	}
 
 	/* Do an implicit "commit" when using the classic CLI mode. */
 	if (frr_get_cli_mode() == FRR_CLI_CLASSIC) {
 		struct nb_context context = {};
+		char errmsg[BUFSIZ] = {0};
 
 		context.client = NB_CLIENT_CLI;
 		context.user = vty;
 		ret = nb_candidate_commit(&context, vty->candidate_config,
-					  false, NULL, NULL);
+					  false, NULL, NULL, errmsg,
+					  sizeof(errmsg));
 		if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) {
-			vty_out(vty, "%% Configuration failed: %s.\n\n",
-				nb_err_name(ret));
-			vty_out(vty,
-				"Please check the logs for more details.\n");
+			vty_out(vty, "%% Configuration failed.\n\n");
+			vty_show_nb_errors(vty, ret, errmsg);
 
 			/* Regenerate candidate for consistency. */
 			nb_config_replace(vty->candidate_config, running_config,
@@ -223,22 +214,25 @@ int nb_cli_confirmed_commit_rollback(struct vty *vty)
 {
 	struct nb_context context = {};
 	uint32_t transaction_id;
+	char errmsg[BUFSIZ] = {0};
 	int ret;
 
-
 	/* Perform the rollback. */
 	context.client = NB_CLIENT_CLI;
 	context.user = vty;
 	ret = nb_candidate_commit(
 		&context, vty->confirmed_commit_rollback, true,
 		"Rollback to previous configuration - confirmed commit has timed out",
-		&transaction_id);
+		&transaction_id, errmsg, sizeof(errmsg));
 	if (ret == NB_OK)
 		vty_out(vty,
 			"Rollback performed successfully (Transaction ID #%u).\n",
 			transaction_id);
-	else
-		vty_out(vty, "Failed to rollback to previous configuration.\n");
+	else {
+		vty_out(vty,
+			"Failed to rollback to previous configuration.\n\n");
+		vty_show_nb_errors(vty, ret, errmsg);
+	}
 
 	return ret;
 }
@@ -262,6 +256,7 @@ static int nb_cli_commit(struct vty *vty, bool force,
 {
 	struct nb_context context = {};
 	uint32_t transaction_id = 0;
+	char errmsg[BUFSIZ] = {0};
 	int ret;
 
 	/* Check if there's a pending confirmed commit. */
@@ -307,7 +302,8 @@ static int nb_cli_commit(struct vty *vty, bool force,
 	context.client = NB_CLIENT_CLI;
 	context.user = vty;
 	ret = nb_candidate_commit(&context, vty->candidate_config, true,
-				  comment, &transaction_id);
+				  comment, &transaction_id, errmsg,
+				  sizeof(errmsg));
 
 	/* Map northbound return code to CLI return code. */
 	switch (ret) {
@@ -323,9 +319,8 @@ static int nb_cli_commit(struct vty *vty, bool force,
 		return CMD_SUCCESS;
 	default:
 		vty_out(vty,
-			"%% Failed to commit candidate configuration: %s.\n\n",
-			nb_err_name(ret));
-		vty_out(vty, "Please check the logs for more details.\n");
+			"%% Failed to commit candidate configuration.\n\n");
+		vty_show_nb_errors(vty, ret, errmsg);
 		return CMD_WARNING;
 	}
 }
@@ -339,6 +334,7 @@ static int nb_cli_candidate_load_file(struct vty *vty,
 	struct lyd_node *dnode;
 	struct ly_ctx *ly_ctx;
 	int ly_format;
+	char buf[BUFSIZ];
 
 	switch (format) {
 	case NB_CFG_FMT_CMDS:
@@ -361,7 +357,9 @@ static int nb_cli_candidate_load_file(struct vty *vty,
 			flog_warn(EC_LIB_LIBYANG, "%s: lyd_parse_path() failed",
 				  __func__);
 			vty_out(vty, "%% Failed to load configuration:\n\n");
-			vty_show_libyang_errors(vty, ly_ctx);
+			vty_out(vty, "%s",
+				yang_print_errors(ly_native_ctx, buf,
+						  sizeof(buf)));
 			return CMD_WARNING;
 		}
 		if (translator
@@ -382,7 +380,8 @@ static int nb_cli_candidate_load_file(struct vty *vty,
 		 != NB_OK) {
 		vty_out(vty,
 			"%% Failed to merge the loaded configuration:\n\n");
-		vty_show_libyang_errors(vty, ly_native_ctx);
+		vty_out(vty, "%s",
+			yang_print_errors(ly_native_ctx, buf, sizeof(buf)));
 		return CMD_WARNING;
 	}
 
@@ -394,6 +393,7 @@ static int nb_cli_candidate_load_transaction(struct vty *vty,
 					     bool replace)
 {
 	struct nb_config *loaded_config;
+	char buf[BUFSIZ];
 
 	loaded_config = nb_db_transaction_load(transaction_id);
 	if (!loaded_config) {
@@ -408,7 +408,8 @@ static int nb_cli_candidate_load_transaction(struct vty *vty,
 		 != NB_OK) {
 		vty_out(vty,
 			"%% Failed to merge the loaded configuration:\n\n");
-		vty_show_libyang_errors(vty, ly_native_ctx);
+		vty_out(vty, "%s",
+			yang_print_errors(ly_native_ctx, buf, sizeof(buf)));
 		return CMD_WARNING;
 	}
 
@@ -702,15 +703,17 @@ DEFPY (config_commit_check,
        "Check if the configuration changes are valid\n")
 {
 	struct nb_context context = {};
+	char errmsg[BUFSIZ] = {0};
 	int ret;
 
 	context.client = NB_CLIENT_CLI;
 	context.user = vty;
-	ret = nb_candidate_validate(&context, vty->candidate_config);
+	ret = nb_candidate_validate(&context, vty->candidate_config, errmsg,
+				    sizeof(errmsg));
 	if (ret != NB_OK) {
 		vty_out(vty,
 			"%% Failed to validate candidate configuration.\n\n");
-		vty_show_libyang_errors(vty, ly_native_ctx);
+		vty_show_nb_errors(vty, ret, errmsg);
 		return CMD_WARNING;
 	}
 
@@ -1547,6 +1550,7 @@ static int nb_cli_rollback_configuration(struct vty *vty,
 	struct nb_context context = {};
 	struct nb_config *candidate;
 	char comment[80];
+	char errmsg[BUFSIZ] = {0};
 	int ret;
 
 	candidate = nb_db_transaction_load(transaction_id);
@@ -1561,7 +1565,8 @@ static int nb_cli_rollback_configuration(struct vty *vty,
 
 	context.client = NB_CLIENT_CLI;
 	context.user = vty;
-	ret = nb_candidate_commit(&context, candidate, true, comment, NULL);
+	ret = nb_candidate_commit(&context, candidate, true, comment, NULL,
+				  errmsg, sizeof(errmsg));
 	nb_config_free(candidate);
 	switch (ret) {
 	case NB_OK:
@@ -1574,7 +1579,7 @@ static int nb_cli_rollback_configuration(struct vty *vty,
 		return CMD_WARNING;
 	default:
 		vty_out(vty, "%% Rollback failed.\n\n");
-		vty_out(vty, "Please check the logs for more details.\n");
+		vty_show_nb_errors(vty, ret, errmsg);
 		return CMD_WARNING;
 	}
 }
diff --git a/lib/northbound_confd.c b/lib/northbound_confd.c
index 50daa62e5a..8d8944aeab 100644
--- a/lib/northbound_confd.c
+++ b/lib/northbound_confd.c
@@ -288,6 +288,7 @@ static int frr_confd_cdb_read_cb_prepare(int fd, int *subp, int reslen)
 	struct nb_context context = {};
 	struct nb_config *candidate;
 	struct cdb_iter_args iter_args;
+	char errmsg[BUFSIZ] = {0};
 	int ret;
 
 	candidate = nb_config_dup(running_config);
@@ -324,23 +325,19 @@ static int frr_confd_cdb_read_cb_prepare(int fd, int *subp, int reslen)
 	transaction = NULL;
 	context.client = NB_CLIENT_CONFD;
 	ret = nb_candidate_commit_prepare(&context, candidate, NULL,
-					  &transaction);
+					  &transaction, errmsg, sizeof(errmsg));
 	if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) {
 		enum confd_errcode errcode;
-		const char *errmsg;
 
 		switch (ret) {
 		case NB_ERR_LOCKED:
 			errcode = CONFD_ERRCODE_IN_USE;
-			errmsg = "Configuration is locked by another process";
 			break;
 		case NB_ERR_RESOURCE:
 			errcode = CONFD_ERRCODE_RESOURCE_DENIED;
-			errmsg = "Failed do allocate resources";
 			break;
 		default:
-			errcode = CONFD_ERRCODE_INTERNAL;
-			errmsg = "Internal error";
+			errcode = CONFD_ERRCODE_APPLICATION;
 			break;
 		}
 
diff --git a/lib/northbound_grpc.cpp b/lib/northbound_grpc.cpp
index b038d10118..5dbfe877f0 100644
--- a/lib/northbound_grpc.cpp
+++ b/lib/northbound_grpc.cpp
@@ -674,17 +674,20 @@ class NorthboundImpl
 			// Execute the user request.
 			struct nb_context context = {};
 			context.client = NB_CLIENT_GRPC;
+			char errmsg[BUFSIZ] = {0};
 
 			switch (phase) {
 			case frr::CommitRequest::VALIDATE:
-				ret = nb_candidate_validate(&context,
-							    candidate->config);
+				ret = nb_candidate_validate(
+					&context, candidate->config, errmsg,
+					sizeof(errmsg));
 				break;
 			case frr::CommitRequest::PREPARE:
 				ret = nb_candidate_commit_prepare(
 					&context, candidate->config,
 					comment.c_str(),
-					&candidate->transaction);
+					&candidate->transaction, errmsg,
+					sizeof(errmsg));
 				break;
 			case frr::CommitRequest::ABORT:
 				nb_candidate_commit_abort(
@@ -698,70 +701,50 @@ class NorthboundImpl
 			case frr::CommitRequest::ALL:
 				ret = nb_candidate_commit(
 					&context, candidate->config, true,
-					comment.c_str(), &transaction_id);
+					comment.c_str(), &transaction_id,
+					errmsg, sizeof(errmsg));
 				break;
 			}
 
-			// Map northbound error codes to gRPC error codes.
+			// Map northbound error codes to gRPC status codes.
+			grpc::Status status;
 			switch (ret) {
+			case NB_OK:
+				status = grpc::Status::OK;
+				break;
 			case NB_ERR_NO_CHANGES:
-				tag->responder.Finish(
-					tag->response,
-					grpc::Status(
-						grpc::StatusCode::ABORTED,
-						"No configuration changes detected"),
-					tag);
-				tag->state = FINISH;
-				return;
+				status = grpc::Status(grpc::StatusCode::ABORTED,
+						      errmsg);
+				break;
 			case NB_ERR_LOCKED:
-				tag->responder.Finish(
-					tag->response,
-					grpc::Status(
-						grpc::StatusCode::UNAVAILABLE,
-						"There's already a transaction in progress"),
-					tag);
-				tag->state = FINISH;
-				return;
+				status = grpc::Status(
+					grpc::StatusCode::UNAVAILABLE, errmsg);
+				break;
 			case NB_ERR_VALIDATION:
-				tag->responder.Finish(
-					tag->response,
-					grpc::Status(grpc::StatusCode::
-							     INVALID_ARGUMENT,
-						     "Validation error"),
-					tag);
-				tag->state = FINISH;
-				return;
+				status = grpc::Status(
+					grpc::StatusCode::INVALID_ARGUMENT,
+					errmsg);
+				break;
 			case NB_ERR_RESOURCE:
-				tag->responder.Finish(
-					tag->response,
-					grpc::Status(
-						grpc::StatusCode::
-							RESOURCE_EXHAUSTED,
-						"Failed do allocate resources"),
-					tag);
-				tag->state = FINISH;
-				return;
+				status = grpc::Status(
+					grpc::StatusCode::RESOURCE_EXHAUSTED,
+					errmsg);
+				break;
 			case NB_ERR:
-				tag->responder.Finish(
-					tag->response,
-					grpc::Status(grpc::StatusCode::INTERNAL,
-						     "Internal error"),
-					tag);
-				tag->state = FINISH;
-				return;
 			default:
+				status = grpc::Status(
+					grpc::StatusCode::INTERNAL, errmsg);
 				break;
 			}
+			if (ret == NB_OK) {
+				// Response: uint32 transaction_id = 1;
+				if (transaction_id)
+					tag->response.set_transaction_id(
+						transaction_id);
+			}
 
-			// Response: uint32 transaction_id = 1;
-			if (transaction_id)
-				tag->response.set_transaction_id(
-					transaction_id);
-
-			tag->responder.Finish(tag->response, grpc::Status::OK,
-					      tag);
+			tag->responder.Finish(tag->response, status, tag);
 			tag->state = FINISH;
-
 			break;
 		}
 		case FINISH:
diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c
index 4d15c80a2b..500203173c 100644
--- a/lib/northbound_sysrepo.c
+++ b/lib/northbound_sysrepo.c
@@ -247,6 +247,7 @@ static int frr_sr_config_change_cb_verify(sr_session_ctx_t *session,
 	char xpath[XPATH_MAXLEN];
 	struct nb_context context = {};
 	struct nb_config *candidate;
+	char errmsg[BUFSIZ] = {0};
 
 	snprintf(xpath, sizeof(xpath), "/%s:*", module_name);
 	ret = sr_get_changes_iter(session, xpath, &it);
@@ -284,15 +285,26 @@ static int frr_sr_config_change_cb_verify(sr_session_ctx_t *session,
 		 * single event (SR_EV_ENABLED). This means we need to perform
 		 * the full two-phase commit protocol in one go here.
 		 */
-		ret = nb_candidate_commit(&context, candidate, true, NULL,
-					  NULL);
+		ret = nb_candidate_commit(&context, candidate, true, NULL, NULL,
+					  errmsg, sizeof(errmsg));
+		if (ret != NB_OK && ret != NB_ERR_NO_CHANGES)
+			flog_warn(
+				EC_LIB_LIBSYSREPO,
+				"%s: failed to apply startup configuration: %s (%s)",
+				__func__, nb_err_name(ret), errmsg);
 	} else {
 		/*
 		 * Validate the configuration changes and allocate all resources
 		 * required to apply them.
 		 */
 		ret = nb_candidate_commit_prepare(&context, candidate, NULL,
-						  &transaction);
+						  &transaction, errmsg,
+						  sizeof(errmsg));
+		if (ret != NB_OK && ret != NB_ERR_NO_CHANGES)
+			flog_warn(
+				EC_LIB_LIBSYSREPO,
+				"%s: failed to prepare configuration transaction: %s (%s)",
+				__func__, nb_err_name(ret), errmsg);
 	}
 
 	/* Map northbound return code to sysrepo return code. */
diff --git a/lib/vty.c b/lib/vty.c
index 146666ec6d..ffef05e4dc 100644
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -2350,14 +2350,17 @@ static void vty_read_file(struct nb_config *config, FILE *confp)
 	 */
 	if (config == NULL) {
 		struct nb_context context = {};
+		char errmsg[BUFSIZ] = {0};
 
 		context.client = NB_CLIENT_CLI;
 		context.user = vty;
 		ret = nb_candidate_commit(&context, vty->candidate_config, true,
-					  "Read configuration file", NULL);
+					  "Read configuration file", NULL,
+					  errmsg, sizeof(errmsg));
 		if (ret != NB_OK && ret != NB_ERR_NO_CHANGES)
-			zlog_err("%s: failed to read configuration file.",
-				 __func__);
+			zlog_err(
+				"%s: failed to read configuration file: %s (%s)",
+				__func__, nb_err_name(ret), errmsg);
 	}
 
 	vty_close(vty);
diff --git a/lib/yang.c b/lib/yang.c
index c80bf20306..5fca686c5a 100644
--- a/lib/yang.c
+++ b/lib/yang.c
@@ -621,6 +621,34 @@ static void ly_log_cb(LY_LOG_LEVEL level, const char *msg, const char *path)
 		zlog(priority, "libyang: %s", msg);
 }
 
+const char *yang_print_errors(struct ly_ctx *ly_ctx, char *buf, size_t buf_len)
+{
+	struct ly_err_item *ei;
+	const char *path;
+
+	ei = ly_err_first(ly_ctx);
+	if (!ei)
+		return "";
+
+	strlcpy(buf, "YANG error(s):\n", buf_len);
+	for (; ei; ei = ei->next) {
+		strlcat(buf, " ", buf_len);
+		strlcat(buf, ei->msg, buf_len);
+		strlcat(buf, "\n", buf_len);
+	}
+
+	path = ly_errpath(ly_ctx);
+	if (path) {
+		strlcat(buf, " YANG path: ", buf_len);
+		strlcat(buf, path, buf_len);
+		strlcat(buf, "\n", buf_len);
+	}
+
+	ly_err_clean(ly_ctx, NULL);
+
+	return buf;
+}
+
 void yang_debugging_set(bool enable)
 {
 	if (enable) {
diff --git a/lib/yang.h b/lib/yang.h
index 126521707b..2f3017e8ad 100644
--- a/lib/yang.h
+++ b/lib/yang.h
@@ -497,6 +497,24 @@ extern struct ly_ctx *yang_ctx_new_setup(bool embedded_modules);
 extern void yang_debugging_set(bool enable);
 
 /*
+ * Print libyang error messages into the provided buffer.
+ *
+ * ly_ctx
+ *    libyang context to operate on.
+ *
+ * buf
+ *    Buffer to store the libyang error messages.
+ *
+ * buf_len
+ *    Size of buf.
+ *
+ * Returns:
+ *    The provided buffer.
+ */
+extern const char *yang_print_errors(struct ly_ctx *ly_ctx, char *buf,
+				     size_t buf_len);
+
+/*
  * Initialize the YANG subsystem. Should be called only once during the
  * daemon initialization process.
  *