cli: improve error logging for call command

When the ubus "call" command fails, ubus prints "Command failed: ...".
This is fine when the call command is executed interactively by the
user. However, when the call command is executed non-interactively,
these log messages leave the user completely clueless:

  netifd: wan6 (10841): Command failed: Unknown error

  procd: /etc/rc.d/S80umdns: Failed to parse message data

These messages contain absolutely no info that explains where they come
from; it's not even clear they are coming from ubus. This makes tracking
down what's causing them virtually impossible.

Improve this situation by printing the full ubus call to stderr when
stderr is not a TTY.

Example of improved error message:
  netifd: wan (2836): Command failed: ubus call network.interface notify_proto { "action": 0, "link-up": false, "keep": false, "interface": "wan" } (Permission denied)

While one might argue not to include the JSON message in the log,
excluding it will still not help to reproduce the failed command.
As we only print the full command when stderr is not a TTY, seeing this
error means we're doing a bad ubus call somewhere, which is a bug and
should be fixed. Printing the full command makes more sense than
printing half the command and still requiring us to figure out the exact
command that failed.

Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
Acked-by: Petr Štetiar <ynezz@true.cz>
Acked-by: Jo-Philipp Wich <jo@mein.io>

diff --git a/cli.c b/cli.c
index 3b67047fa9e81e3641ca175a8ab35640338eed13..5d2cd4c9cfbcec341028f41dfb480ab4487ca29e 100644 (file)
--- a/cli.c
+++ b/cli.c
@@ -119,6 +119,23 @@ static void receive_event(struct ubus_context *ctx, struct ubus_event_handler *e
        print_event(type, msg);
 }
 
+static int ubus_cli_error(char *cmd, int argc, char **argv, int err)
+{
+       int i;
+
+       if (!simple_output && !isatty(fileno(stderr))) {
+              fprintf(stderr, "Command failed: ubus %s ", cmd);
+              for (i = 0; i < argc; i++) {
+                      fprintf(stderr, "%s ", argv[i]);
+              }
+              fprintf(stderr, "(%s)\n", ubus_strerror(err));
+
+              return -err;
+       }
+
+       return err;
+}
+
 static int ubus_cli_list(struct ubus_context *ctx, int argc, char **argv)
 {
        const char *path = NULL;
@@ -142,14 +159,18 @@ static int ubus_cli_call(struct ubus_context *ctx, int argc, char **argv)
 
        blob_buf_init(&b, 0);
        if (argc == 3 && !blobmsg_add_json_from_string(&b, argv[2])) {
-               return UBUS_STATUS_PARSE_ERROR;
+               return ubus_cli_error("call", argc, argv, UBUS_STATUS_PARSE_ERROR);
        }
 
        ret = ubus_lookup_id(ctx, argv[0], &id);
        if (ret)
                return ret;
 
-       return ubus_invoke(ctx, id, argv[1], b.head, receive_call_result_data, NULL, timeout * 1000);
+       ret = ubus_invoke(ctx, id, argv[1], b.head, receive_call_result_data, NULL, timeout * 1000);
+       if (ret)
+               return ubus_cli_error("call", argc, argv, ret);
+
+       return ret;
 }
 
 struct cli_listen_data {