Merge PR #1199 'Improvements to API error handling'

3 files changed, 49 insertions, 50 deletions

diff --git a/src/nvim/os/channel.c b/src/nvim/os/channel.c
index 1670424e4e..5598b485ba 100644
--- a/src/nvim/os/channel.c
+++ b/src/nvim/os/channel.c
@@ -172,40 +172,36 @@ bool channel_send_event(uint64_t id, char *name, Array args)
 /// Sends a method call to a channel
 ///
 /// @param id The channel id
-/// @param name The method name, an arbitrary string
+/// @param method_name The method name, an arbitrary string
 /// @param args Array with method arguments
-/// @param[out] result Pointer to return value received from the channel
 /// @param[out] error True if the return value is an error
-/// @return True if the call was sent successfully, false otherwise.
-bool channel_send_call(uint64_t id,
-                       char *name,
-                       Array args,
-                       Object *result,
-                       bool *errored)
+/// @return Whatever the remote method returned
+Object channel_send_call(uint64_t id,
+                         char *method_name,
+                         Array args,
+                         Error *err)
 {
   Channel *channel = NULL;
 
   if (!(channel = pmap_get(uint64_t)(channels, id)) || !channel->enabled) {
+    api_set_error(err, Exception, _("Invalid channel \"%" PRIu64 "\""), id);
     api_free_array(args);
-    return false;
+    return NIL;
   }
 
   if (kv_size(channel->call_stack) > 20) {
     // 20 stack depth is more than anyone should ever need for RPC calls
-    *errored = true;
-    char buf[256];
-    snprintf(buf,
-        sizeof(buf),
-        "Channel %" PRIu64 " crossed maximum stack depth",
-        channel->id);
-    *result = STRING_OBJ(cstr_to_string(buf));
+    api_set_error(err,
+                  Exception,
+                  _("Channel %" PRIu64 " crossed maximum stack depth"),
+                  channel->id);
     api_free_array(args);
-    return false;
+    return NIL;
   }
 
   uint64_t request_id = channel->next_request_id++;
   // Send the msgpack-rpc request
-  send_request(channel, request_id, name, args);
+  send_request(channel, request_id, method_name, args);
 
   EventSource channel_source = channel->is_job
     ? job_event_source(channel->data.job)
@@ -224,10 +220,12 @@ bool channel_send_call(uint64_t id,
       channel->enabled &&  // the channel is still enabled
       kv_size(channel->call_stack) >= size);  // the call didn't return
 
-  *errored = frame.errored;
-  *result = frame.result;
+  if (frame.errored) {
+    api_set_error(err, Exception, "%s", frame.result.data.string.data);
+    return NIL;
+  }
 
-  return true;
+  return frame.result;
 }
 
 /// Subscribes to event broadcasts
@@ -433,7 +431,9 @@ static bool channel_write(Channel *channel, WBuffer *buffer)
 
 static void send_error(Channel *channel, uint64_t id, char *err)
 {
-  channel_write(channel, serialize_response(id, err, NIL, &out_buffer));
+  Error e = ERROR_INIT;
+  api_set_error(&e, Exception, "%s", err);
+  channel_write(channel, serialize_response(id, &e, NIL, &out_buffer));
 }
 
 static void send_request(Channel *channel,
diff --git a/src/nvim/os/msgpack_rpc.c b/src/nvim/os/msgpack_rpc.c
index d7e3d33c4b..55bc006ad1 100644
--- a/src/nvim/os/msgpack_rpc.c
+++ b/src/nvim/os/msgpack_rpc.c
@@ -30,14 +30,14 @@ WBuffer *msgpack_rpc_call(uint64_t channel_id,
   FUNC_ATTR_NONNULL_ARG(3)
 {
   uint64_t response_id;
-  char *err = msgpack_rpc_validate(&response_id, req);
+  Error error = ERROR_INIT;
+  msgpack_rpc_validate(&response_id, req, &error);
 
-  if (err) {
-    return serialize_response(response_id, err, NIL, sbuffer);
+  if (error.set) {
+    return serialize_response(response_id, &error, NIL, sbuffer);
   }
 
   // dispatch the call
-  Error error = { .set = false };
   Object rv = msgpack_rpc_dispatch(channel_id, req, &error);
   // send the response
   msgpack_packer response;
@@ -47,12 +47,12 @@ WBuffer *msgpack_rpc_call(uint64_t channel_id,
     ELOG("Error dispatching msgpack-rpc call: %s(request: id %" PRIu64 ")",
          error.msg,
          response_id);
-    return serialize_response(response_id, error.msg, NIL, sbuffer);
+    return serialize_response(response_id, &error, NIL, sbuffer);
   }
 
   DLOG("Successfully completed mspgack-rpc call(request id: %" PRIu64 ")",
        response_id);
-  return serialize_response(response_id, NULL, rv, sbuffer);
+  return serialize_response(response_id, &error, rv, sbuffer);
 }
 
 /// Finishes the msgpack-rpc call with an error message.
@@ -112,10 +112,10 @@ WBuffer *serialize_request(uint64_t request_id,
 
 /// Serializes a msgpack-rpc response
 WBuffer *serialize_response(uint64_t response_id,
-                            char *err_msg,
+                            Error *err,
                             Object arg,
                             msgpack_sbuffer *sbuffer)
-  FUNC_ATTR_NONNULL_ARG(4)
+  FUNC_ATTR_NONNULL_ARG(2, 4)
 {
   msgpack_packer pac;
   msgpack_packer_init(&pac, sbuffer, msgpack_sbuffer_write);
@@ -123,11 +123,11 @@ WBuffer *serialize_response(uint64_t response_id,
   msgpack_pack_int(&pac, 1);
   msgpack_pack_uint64(&pac, response_id);
 
-  if (err_msg) {
-    String err = {.size = strlen(err_msg), .data = err_msg};
-    // error message
-    msgpack_pack_bin(&pac, err.size);
-    msgpack_pack_bin_body(&pac, err.data, err.size);
+  if (err->set) {
+    // error represented by a [type, message] array
+    msgpack_pack_array(&pac, 2);
+    msgpack_rpc_from_integer(err->type, &pac);
+    msgpack_rpc_from_string(cstr_as_string(err->msg), &pac);
     // Nil result
     msgpack_pack_nil(&pac);
   } else {
@@ -146,43 +146,43 @@ WBuffer *serialize_response(uint64_t response_id,
   return rv;
 }
 
-static char *msgpack_rpc_validate(uint64_t *response_id, msgpack_object *req)
+static void msgpack_rpc_validate(uint64_t *response_id,
+                                 msgpack_object *req,
+                                 Error *err)
 {
   // response id not known yet
 
   *response_id = 0;
   // Validate the basic structure of the msgpack-rpc payload
   if (req->type != MSGPACK_OBJECT_ARRAY) {
-    return "Request is not an array";
+    api_set_error(err, Validation, _("Request is not an array"));
   }
 
   if (req->via.array.size != 4) {
-    return "Request array size should be 4";
+    api_set_error(err, Validation, _("Request array size should be 4"));
   }
 
   if (req->via.array.ptr[1].type != MSGPACK_OBJECT_POSITIVE_INTEGER) {
-    return "Id must be a positive integer";
+    api_set_error(err, Validation, _("Id must be a positive integer"));
   }
 
   // Set the response id, which is the same as the request
   *response_id = req->via.array.ptr[1].via.u64;
 
   if (req->via.array.ptr[0].type != MSGPACK_OBJECT_POSITIVE_INTEGER) {
-    return "Message type must be an integer";
+    api_set_error(err, Validation, _("Message type must be an integer"));
   }
 
   if (req->via.array.ptr[0].via.u64 != 0) {
-    return "Message type must be 0";
+    api_set_error(err, Validation, _("Message type must be 0"));
   }
 
   if (req->via.array.ptr[2].type != MSGPACK_OBJECT_BIN
     && req->via.array.ptr[2].type != MSGPACK_OBJECT_STR) {
-    return "Method must be a string";
+    api_set_error(err, Validation, _("Method must be a string"));
   }
 
   if (req->via.array.ptr[3].type != MSGPACK_OBJECT_ARRAY) {
-    return "Paremeters must be an array";
+    api_set_error(err, Validation, _("Paremeters must be an array"));
   }
-
-  return NULL;
 }
diff --git a/src/nvim/os/provider.c b/src/nvim/os/provider.c
index 2e7a677793..d4fffaa053 100644
--- a/src/nvim/os/provider.c
+++ b/src/nvim/os/provider.c
@@ -107,12 +107,11 @@ Object provider_call(char *method, Array args)
     return NIL;
   }
 
-  bool error = false;
-  Object result = NIL;
-  channel_send_call(f->channel_id, method, args, &result, &error);
+  Error err = ERROR_INIT;
+  Object result = NIL = channel_send_call(f->channel_id, method, args, &err);
 
-  if (error) {
-    vim_report_error(result.data.string);
+  if (err.set) {
+    vim_report_error(cstr_as_string(err.msg));
     api_free_object(result);
     return NIL;
   }