Allow embedder service request handlers to return JSON-RPC errors.

BUG=
R=johnmccutchan@google.com

Review URL: https://codereview.chromium.org/2041293003 .

runtime/bin/main.cc

@@ -935,9 +935,11 @@ static void ShutdownIsolate(void* callback_data) {
 }
 
 
-static const char* ServiceRequestError(Dart_Handle error) {
+static const char* InternalJsonRpcError(Dart_Handle error) {
   TextBuffer buffer(128);
-  buffer.Printf("{\"type\":\"Error\",\"text\":\"Internal error %s\"}",
+  buffer.Printf("{\"code\":-32603,"
+                "\"message\":\"Internal error\","
+                "\"details\": \"%s\"}",
                 Dart_GetError(error));
   return buffer.Steal();
 }
@@ -950,28 +952,32 @@ class DartScope {
 };
 
 
-static const char* ServiceGetIOHandler(
+static bool ServiceGetIOHandler(
     const char* method,
     const char** param_keys,
     const char** param_values,
     intptr_t num_params,
-    void* user_data) {
+    void* user_data,
+    const char** response) {
   DartScope scope;
   // TODO(ajohnsen): Store the library/function in isolate data or user_data.
   Dart_Handle dart_io_str = Dart_NewStringFromCString("dart:io");
   if (Dart_IsError(dart_io_str)) {
-    return ServiceRequestError(dart_io_str);
+    *response = InternalJsonRpcError(dart_io_str);
+    return false;
   }
 
   Dart_Handle io_lib = Dart_LookupLibrary(dart_io_str);
   if (Dart_IsError(io_lib)) {
-    return ServiceRequestError(io_lib);
+    *response = InternalJsonRpcError(io_lib);
+    return false;
   }
 
   Dart_Handle handler_function_name =
       Dart_NewStringFromCString("_serviceObjectHandler");
   if (Dart_IsError(handler_function_name)) {
-    return ServiceRequestError(handler_function_name);
+    *response = InternalJsonRpcError(handler_function_name);
+    return false;
   }
 
   // TODO(johnmccutchan): paths is no longer used.  Update the io
@@ -986,15 +992,18 @@ static const char* ServiceGetIOHandler(
   Dart_Handle args[] = {paths, keys, values};
   Dart_Handle result = Dart_Invoke(io_lib, handler_function_name, 3, args);
   if (Dart_IsError(result)) {
-    return ServiceRequestError(result);
+    *response = InternalJsonRpcError(result);
+    return false;
   }
 
   const char *json;
   result = Dart_StringToCString(result, &json);
   if (Dart_IsError(result)) {
-    return ServiceRequestError(result);
+    *response = InternalJsonRpcError(result);
+    return false;
   }
-  return strdup(json);
+  *response = strdup(json);
+  return true;
 }
 
 

runtime/include/dart_tools_api.h

@@ -752,22 +752,44 @@ DART_EXPORT Dart_IsolateId Dart_GetIsolateId(Dart_Isolate isolate);
  * a service request it can't handle and the service request command name
  * matches one of the embedder registered handlers.
  *
+ * The return value of the callback indicates whether the response
+ * should be used as a regular result or an error result.
+ * Specifically, if the callback returns true, a regular JSON-RPC
+ * response is built in the following way:
+ *
+ * {
+ *   "jsonrpc": "2.0",
+ *   "result": <json_object>,
+ *   "id": <some sequence id>,
+ * }
+ *
+ * If the callback returns false, a JSON-RPC error is built like this:
+ *
+ * {
+ *   "jsonrpc": "2.0",
+ *   "error": <json_object>,
+ *   "id": <some sequence id>,
+ * }
+ *
  * \param method The rpc method name.
  * \param param_keys Service requests can have key-value pair parameters. The
  *   keys and values are flattened and stored in arrays.
  * \param param_values The values associated with the keys.
  * \param num_params The length of the param_keys and param_values arrays.
  * \param user_data The user_data pointer registered with this handler.
+ * \param result A C string containing a valid JSON object. The returned
+ *   pointer will be freed by the VM by calling free.
  *
- * \return Returns a C string containing a valid JSON object. The returned
- * pointer will be freed by the VM by calling free.
+ * \return True if the result is a regular JSON-RPC response, false if the
+ *   result is a JSON-RPC error.
  */
-typedef const char* (*Dart_ServiceRequestCallback)(
+typedef bool (*Dart_ServiceRequestCallback)(
     const char* method,
     const char** param_keys,
     const char** param_values,
     intptr_t num_params,
-    void* user_data);
+    void* user_data,
+    const char** json_object);
 
 
 /**

runtime/vm/service.cc

@@ -859,7 +859,6 @@ void Service::InvokeMethod(Isolate* I, const Array& msg) {
 
     if (handler != NULL) {
       EmbedderHandleMessage(handler, &js);
-      js.PostReply();
       return;
     }
 
@@ -1126,16 +1125,16 @@ void Service::EmbedderHandleMessage(EmbedderServiceHandler* handler,
   ASSERT(handler != NULL);
   Dart_ServiceRequestCallback callback = handler->callback();
   ASSERT(callback != NULL);
-  const char* r = NULL;
-  const char* method = js->method();
-  const char** keys = js->param_keys();
-  const char** values = js->param_values();
-  r = callback(method, keys, values, js->num_params(), handler->user_data());
-  ASSERT(r != NULL);
-  // TODO(johnmccutchan): Allow for NULL returns?
-  TextBuffer* buffer = js->buffer();
-  buffer->AddString(r);
-  free(const_cast<char*>(r));
+  const char* response = NULL;
+  bool success = callback(js->method(), js->param_keys(), js->param_values(),
+                          js->num_params(), handler->user_data(), &response);
+  ASSERT(response != NULL);
+  if (!success) {
+    js->SetupError();
+  }
+  js->buffer()->AddString(response);
+  js->PostReply();
+  free(const_cast<char*>(response));
 }
 
 

runtime/vm/service_test.cc

@@ -571,23 +571,27 @@ TEST_CASE(Service_Address) {
 }
 
 
-static const char* alpha_callback(
+static bool alpha_callback(
     const char* name,
     const char** option_keys,
     const char** option_values,
     intptr_t num_options,
-    void* user_data) {
-  return strdup("alpha");
+    void* user_data,
+    const char** result) {
+  *result = strdup("alpha");
+  return true;
 }
 
 
-static const char* beta_callback(
+static bool beta_callback(
     const char* name,
     const char** option_keys,
     const char** option_values,
     intptr_t num_options,
-    void* user_data) {
-  return strdup("beta");
+    void* user_data,
+    const char** result) {
+  *result = strdup("beta");
+  return false;
 }
 
 
@@ -626,7 +630,7 @@ TEST_CASE(Service_EmbedderRootHandler) {
   service_msg = Eval(lib, "[0, port, 1, 'beta', [], []]");
   Service::HandleRootMessage(service_msg);
   EXPECT_EQ(MessageHandler::kOK, handler.HandleNextMessage());
-  EXPECT_STREQ("{\"jsonrpc\":\"2.0\", \"result\":beta,\"id\":1}",
+  EXPECT_STREQ("{\"jsonrpc\":\"2.0\", \"error\":beta,\"id\":1}",
                handler.msg());
 }
 
@@ -666,7 +670,7 @@ TEST_CASE(Service_EmbedderIsolateHandler) {
   service_msg = Eval(lib, "[0, port, '0', 'beta', [], []]");
   Service::HandleIsolateMessage(isolate, service_msg);
   EXPECT_EQ(MessageHandler::kOK, handler.HandleNextMessage());
-  EXPECT_STREQ("{\"jsonrpc\":\"2.0\", \"result\":beta,\"id\":\"0\"}",
+  EXPECT_STREQ("{\"jsonrpc\":\"2.0\", \"error\":beta,\"id\":\"0\"}",
                handler.msg());
 }