Rework error handling in the service protocol and in Observatory.

runtime/vm/service.cc

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/service.h"
 
 #include "include/dart_api.h"
 #include "platform/globals.h"
 
 #include "vm/compiler.h"
@@ skipping 32 matching lines @@
 EmbedderServiceHandler* Service::isolate_service_handler_head_ = NULL;
 EmbedderServiceHandler* Service::root_service_handler_head_ = NULL;
 struct ServiceMethodDescriptor;
 ServiceMethodDescriptor* FindMethod(const char* method_name);
 
 static uint8_t* allocator(uint8_t* ptr, intptr_t old_size, intptr_t new_size) {
   void* new_ptr = realloc(reinterpret_cast<void*>(ptr), new_size);
   return reinterpret_cast<uint8_t*>(new_ptr);
 }
 
-static void PrintRequest(const JSONObject& obj, JSONStream* js) {
-  JSONObject jsobj(&obj, "request");
-  jsobj.AddProperty("method", js->method());
-  {
-    JSONObject params(&jsobj, "params");
-    for (intptr_t i = 0; i < js->num_params(); i++) {
-      params.AddProperty(js->GetParamKey(i), js->GetParamValue(i));
-    }
-  }
-}
-
-
-static void PrintError(JSONStream* js,
-                       const char* format, ...) {
-  Isolate* isolate = Isolate::Current();
-
-  va_list args;
-  va_start(args, format);
-  intptr_t len = OS::VSNPrint(NULL, 0, format, args);
-  va_end(args);
-
-  char* buffer = isolate->current_zone()->Alloc<char>(len + 1);
-  va_list args2;
-  va_start(args2, format);
-  OS::VSNPrint(buffer, (len + 1), format, args2);
-  va_end(args2);
-
-  JSONObject jsobj(js);
-  jsobj.AddProperty("type", "Error");
-  jsobj.AddProperty("message", buffer);
-  PrintRequest(jsobj, js);
-}
-
-
 static void PrintMissingParamError(JSONStream* js,
                                    const char* param) {
-  PrintError(js, "%s expects the '%s' parameter",
-             js->method(), param);
+  js->PrintError(kInvalidParams,
+                 "%s expects the '%s' parameter", js->method(), param);
 }
 
 
 static void PrintInvalidParamError(JSONStream* js,
                                    const char* param) {
-  PrintError(js, "%s: invalid '%s' parameter: %s",
-             js->method(), param, js->LookupParam(param));
+  js->PrintError(kInvalidParams,
+                 "%s: invalid '%s' parameter: %s",
+                 js->method(), param, js->LookupParam(param));
 }
 
 
 static void PrintUnrecognizedMethodError(JSONStream* js) {
-  PrintError(js, "unrecognized method: %s", js->method());
+  js->PrintError(kMethodNotFound, NULL);
 }
 
 
-static void PrintErrorWithKind(JSONStream* js,
-                               const char* kind,
-                               const char* format, ...) {
-  Isolate* isolate = Isolate::Current();
-
-  va_list args;
-  va_start(args, format);
-  intptr_t len = OS::VSNPrint(NULL, 0, format, args);
-  va_end(args);
-
-  char* buffer = isolate->current_zone()->Alloc<char>(len + 1);
-  va_list args2;
-  va_start(args2, format);
-  OS::VSNPrint(buffer, (len + 1), format, args2);
-  va_end(args2);
-
-  JSONObject jsobj(js);
-  jsobj.AddProperty("type", "Error");
-  jsobj.AddProperty("id", "");
-  jsobj.AddProperty("kind", kind);
-  jsobj.AddProperty("message", buffer);
-  PrintRequest(jsobj, js);
-}
-
-
 static bool GetIntegerId(const char* s, intptr_t* id, int base = 10) {
   if ((s == NULL) || (*s == '\0')) {
     // Empty string.
     return false;
   }
   if (id == NULL) {
     // No id pointer.
     return false;
   }
   intptr_t r = 0;
@@ skipping 1054 matching lines @@
     return false;
   }
   const char* rest = id + end_pos + 1;  // +1 for '/'.
   if (strncmp("messages", id, end_pos) == 0) {
     uword message_id = 0;
     if (GetUnsignedIntegerId(rest, &message_id, 16)) {
       MessageHandler::AcquiredQueues aq;
       isolate->message_handler()->AcquireQueues(&aq);
       Message* message = aq.queue()->FindMessageById(message_id);
       if (message == NULL) {
-        printf("Could not find message %" Px "\n", message_id);
-        // Not found.
-        return false;
+        // The user may try to load an expired message, so we treat
+        // unrecognized ids as if they are expired.
+        PrintSentinel(js, "objects/expired", "<expired>");

[Cutch, 2015/05/13 17:50:10]

Should PrintSentinel take an enum and handle setting the id and preview fields?
Would put all of the constant strings printed below into one place.

[turnidge, 2015/05/14 17:53:43]

Done.

+        return true;
       }
       SnapshotReader reader(message->data(),
                             message->len(),
                             Snapshot::kMessage,
                             isolate,
                             isolate->current_zone());
       const Object& msg_obj = Object::Handle(reader.ReadObject());
       msg_obj.PrintJSON(js);
       return true;
-    } else {
-      printf("Could not get id from %s\n", rest);
     }
   }
   return false;
 }
 
 
 static bool PrintInboundReferences(Isolate* isolate,
                                    Object* target,
                                    intptr_t limit,
                                    JSONStream* js) {
@@ skipping 47 matching lines @@
 };
 
 
 static bool GetInboundReferences(Isolate* isolate, JSONStream* js) {
   const char* target_id = js->LookupParam("targetId");
   if (target_id == NULL) {
     PrintMissingParamError(js, "targetId");
     return true;
   }
   const char* limit_cstr = js->LookupParam("limit");
-  if (target_id == NULL) {
+  if (limit_cstr == NULL) {
     PrintMissingParamError(js, "limit");
     return true;
   }
   intptr_t limit;
   if (!GetIntegerId(limit_cstr, &limit)) {
     PrintInvalidParamError(js, "limit");
     return true;
   }
 
   Object& obj = Object::Handle(isolate);
   ObjectIdRing::LookupResult lookup_result;
   {
     HANDLESCOPE(isolate);
     obj = LookupHeapObject(isolate, target_id, &lookup_result);
   }
   if (obj.raw() == Object::sentinel().raw()) {
     if (lookup_result == ObjectIdRing::kCollected) {
-      PrintErrorWithKind(
-          js, "InboundReferencesCollected",
-          "attempt to find a retaining path for a collected object\n");
-      return true;
+      PrintSentinel(js, "objects/collected", "<collected>");
     } else if (lookup_result == ObjectIdRing::kExpired) {
-      PrintErrorWithKind(
-          js, "InboundReferencesExpired",
-          "attempt to find a retaining path for an expired object\n");
-      return true;
+      PrintSentinel(js, "objects/expired", "<expired>");
+    } else {
+      PrintInvalidParamError(js, "targetId");
     }
-    PrintInvalidParamError(js, "targetId");
     return true;
   }
   return PrintInboundReferences(isolate, &obj, limit, js);
 }
 
 
 static bool PrintRetainingPath(Isolate* isolate,
                                Object* obj,
                                intptr_t limit,
                                JSONStream* js) {
@@ skipping 55 matching lines @@
 };
 
 
 static bool GetRetainingPath(Isolate* isolate, JSONStream* js) {
   const char* target_id = js->LookupParam("targetId");
   if (target_id == NULL) {
     PrintMissingParamError(js, "targetId");
     return true;
   }
   const char* limit_cstr = js->LookupParam("limit");
-  if (target_id == NULL) {
+  if (limit_cstr == NULL) {
     PrintMissingParamError(js, "limit");
     return true;
   }
   intptr_t limit;
   if (!GetIntegerId(limit_cstr, &limit)) {
     PrintInvalidParamError(js, "limit");
     return true;
   }
 
   Object& obj = Object::Handle(isolate);
   ObjectIdRing::LookupResult lookup_result;
   {
     HANDLESCOPE(isolate);
     obj = LookupHeapObject(isolate, target_id, &lookup_result);
   }
   if (obj.raw() == Object::sentinel().raw()) {
     if (lookup_result == ObjectIdRing::kCollected) {
-      PrintErrorWithKind(
-          js, "RetainingPathCollected",
-          "attempt to find a retaining path for a collected object\n");
-      return true;
+      PrintSentinel(js, "objects/collected", "<collected>");
     } else if (lookup_result == ObjectIdRing::kExpired) {
-      PrintErrorWithKind(
-          js, "RetainingPathExpired",
-          "attempt to find a retaining path for an expired object\n");
-      return true;
+      PrintSentinel(js, "objects/expired", "<expired>");
+    } else {
+      PrintInvalidParamError(js, "targetId");
     }
-    PrintInvalidParamError(js, "targetId");
     return true;
   }
   return PrintRetainingPath(isolate, &obj, limit, js);
 }
 
 
 static const MethodParameter* get_retained_size_params[] = {
   ISOLATE_PARAMETER,
   NULL,
 };
 
 
 static bool GetRetainedSize(Isolate* isolate, JSONStream* js) {
   const char* target_id = js->LookupParam("targetId");
   if (target_id == NULL) {
     PrintMissingParamError(js, "targetId");
     return true;
   }
   ObjectIdRing::LookupResult lookup_result;
   Object& obj = Object::Handle(LookupHeapObject(isolate, target_id,
                                                 &lookup_result));
   if (obj.raw() == Object::sentinel().raw()) {
     if (lookup_result == ObjectIdRing::kCollected) {
-      PrintErrorWithKind(
-          js, "RetainedCollected",
-          "attempt to calculate size retained by a collected object\n");
-      return true;
+      PrintSentinel(js, "objects/collected", "<collected>");
     } else if (lookup_result == ObjectIdRing::kExpired) {
-      PrintErrorWithKind(
-          js, "RetainedExpired",
-          "attempt to calculate size retained by an expired object\n");
-      return true;
+      PrintSentinel(js, "objects/expired", "<expired>");
+    } else {
+      PrintInvalidParamError(js, "targetId");
     }
-    PrintInvalidParamError(js, "targetId");
     return true;
   }
   if (obj.IsClass()) {
     const Class& cls = Class::Cast(obj);
     ObjectGraph graph(isolate);
     intptr_t retained_size = graph.SizeRetainedByClass(cls.id());
     const Object& result = Object::Handle(Integer::New(retained_size));
     result.PrintJSON(js, true);
     return true;
   }
   if (obj.IsInstance() || obj.IsNull()) {
     // We don't use Instance::Cast here because it doesn't allow null.
     ObjectGraph graph(isolate);
     intptr_t retained_size = graph.SizeRetainedByInstance(obj);
     const Object& result = Object::Handle(Integer::New(retained_size));
     result.PrintJSON(js, true);
     return true;
   }
-  PrintError(js, "%s: Invalid 'targetId' parameter value: "
-             "id '%s' does not correspond to a "
-             "library, class, or instance", js->method(), target_id);
+  js->PrintError(kInvalidParams,
+                 "%s: invalid 'targetId' parameter: "
+                 "id '%s' does not correspond to a "
+                 "library, class, or instance", js->method(), target_id);
   return true;
 }
 
 
 static const MethodParameter* eval_params[] = {
   ISOLATE_PARAMETER,
   NULL,
 };
 
 
@@ skipping 43 matching lines @@
     // We don't use Instance::Cast here because it doesn't allow null.
     Instance& instance = Instance::Handle(isolate);
     instance ^= obj.raw();
     const Object& result =
         Object::Handle(instance.Evaluate(expr_str,
                                          Array::empty_array(),
                                          Array::empty_array()));
     result.PrintJSON(js, true);
     return true;
   }
-  PrintError(js, "%s: Invalid 'targetId' parameter value: "
-             "id '%s' does not correspond to a "
-             "library, class, or instance", js->method(), target_id);
+  js->PrintError(kInvalidParams,
+                 "%s: invalid 'targetId' parameter: "
+                 "id '%s' does not correspond to a "
+                 "library, class, or instance", js->method(), target_id);
   return true;
 }
 
 
 static const MethodParameter* eval_frame_params[] = {
   ISOLATE_PARAMETER,
   new UIntParameter("frame", true),
   new MethodParameter("expression", true),
   NULL,
 };
@@ skipping 55 matching lines @@
 };
 
 
 static bool GetInstances(Isolate* isolate, JSONStream* js) {
   const char* target_id = js->LookupParam("classId");
   if (target_id == NULL) {
     PrintMissingParamError(js, "classId");
     return true;
   }
   const char* limit_cstr = js->LookupParam("limit");
-  if (target_id == NULL) {
+  if (limit_cstr == NULL) {
     PrintMissingParamError(js, "limit");
     return true;
   }
   intptr_t limit;
   if (!GetIntegerId(limit_cstr, &limit)) {
     PrintInvalidParamError(js, "limit");
     return true;
   }
   const Object& obj =
       Object::Handle(LookupHeapObject(isolate, target_id, NULL));
@@ skipping 110 matching lines @@
   if (obj.IsClass()) {
     ClassCoverageFilter cf(Class::Cast(obj));
     CodeCoverage::PrintJSON(isolate, js, &cf, as_sites);
     return true;
   }
   if (obj.IsFunction()) {
     FunctionCoverageFilter ff(Function::Cast(obj));
     CodeCoverage::PrintJSON(isolate, js, &ff, as_sites);
     return true;
   }
-  PrintError(js, "%s: Invalid 'targetId' parameter value: "
-             "id '%s' does not correspond to a "
-             "script, library, class, or function", js->method(), target_id);
+  js->PrintError(kInvalidParams,
+                 "%s: invalid 'targetId' parameter: "
+                 "id '%s' does not correspond to a "
+                 "script, library, class, or function",
+                 js->method(), target_id);
   return true;
 }
 
 
 static const MethodParameter* get_coverage_params[] = {
   ISOLATE_PARAMETER,
   NULL,
 };
 
 
@@ skipping 30 matching lines @@
   Object& obj = Object::Handle(LookupHeapObject(isolate, script_id, NULL));
   if (obj.raw() == Object::sentinel().raw() || !obj.IsScript()) {
     PrintInvalidParamError(js, "scriptId");
     return true;
   }
   const Script& script = Script::Cast(obj);
   const String& script_url = String::Handle(script.url());
   SourceBreakpoint* bpt =
       isolate->debugger()->SetBreakpointAtLine(script_url, line);
   if (bpt == NULL) {
-    PrintError(js, "Unable to set breakpoint at line %s", line_param);
+    js->PrintError(kNoBreakAtLine, NULL);
     return true;
   }
   bpt->PrintJSON(js);
   return true;
 }
 
 
 static const MethodParameter* add_breakpoint_at_entry_params[] = {
   ISOLATE_PARAMETER,
   new IdParameter("functionId", true),
   NULL,
 };
 
 
 static bool AddBreakpointAtEntry(Isolate* isolate, JSONStream* js) {
   const char* function_id = js->LookupParam("functionId");
   Object& obj = Object::Handle(LookupHeapObject(isolate, function_id, NULL));
   if (obj.raw() == Object::sentinel().raw() || !obj.IsFunction()) {
     PrintInvalidParamError(js, "functionId");
     return true;
   }
   const Function& function = Function::Cast(obj);
   SourceBreakpoint* bpt =
       isolate->debugger()->SetBreakpointAtEntry(function);
   if (bpt == NULL) {
-    const String& funcName = String::Handle(function.PrettyName());
-    PrintError(js, "Unable to set breakpoint at function '%s'",
-               funcName.ToCString());
+    js->PrintError(kNoBreakAtFunction, NULL);
     return true;
   }
   bpt->PrintJSON(js);
   return true;
 }
 
 
 static const MethodParameter* remove_breakpoint_params[] = {
   ISOLATE_PARAMETER,
   NULL,
@@ skipping 57 matching lines @@
   Metric* current = isolate->metrics_list_head();
   while (current != NULL) {
     const char* name = current->name();
     ASSERT(name != NULL);
     if (strcmp(name, id) == 0) {
       current->PrintJSON(js);
       return true;
     }
     current = current->next();
   }
-  PrintError(js, "Native Metric %s not found\n", id);
+  PrintInvalidParamError(js, "metricId");
   return true;
 }
 
 
 static bool HandleDartMetricsList(Isolate* isolate, JSONStream* js) {
   const Class& metrics_cls = Class::Handle(isolate, GetMetricsClass(isolate));
   const String& print_metrics_name =
       String::Handle(String::New("_printMetrics"));
   ASSERT(!print_metrics_name.IsNull());
   const Function& print_metrics = Function::Handle(
@@ skipping 26 matching lines @@
   ASSERT(!args.IsNull());
   args.SetAt(0, arg0);
   const Object& result =
       Object::Handle(isolate, DartEntry::InvokeFunction(print_metric, args));
   if (!result.IsNull()) {
     ASSERT(result.IsString());
     TextBuffer* buffer = js->buffer();
     buffer->AddString(String::Cast(result).ToCString());
     return true;
   }
-  PrintError(js, "Dart Metric %s not found\n", id);
+  PrintInvalidParamError(js, "metricId");
   return true;
 }
 
 
 static const MethodParameter* get_isolate_metric_list_params[] = {
   ISOLATE_PARAMETER,
   NULL,
 };
 
 
@@ skipping 28 matching lines @@
 static bool GetIsolateMetric(Isolate* isolate, JSONStream* js) {
   const char* metric_id = js->LookupParam("metricId");
   if (metric_id == NULL) {
     PrintMissingParamError(js, "metricId");
     return true;
   }
   // Verify id begins with "metrics/".
   static const char* kMetricIdPrefix = "metrics/";
   static intptr_t kMetricIdPrefixLen = strlen(kMetricIdPrefix);
   if (strncmp(metric_id, kMetricIdPrefix, kMetricIdPrefixLen) != 0) {
-    PrintError(js, "Metric %s not found\n", metric_id);
+    PrintInvalidParamError(js, "metricId");
+    return true;
   }
   // Check if id begins with "metrics/native/".
   static const char* kNativeMetricIdPrefix = "metrics/native/";
   static intptr_t kNativeMetricIdPrefixLen = strlen(kNativeMetricIdPrefix);
   const bool native_metric =
       strncmp(metric_id, kNativeMetricIdPrefix, kNativeMetricIdPrefixLen) == 0;
   if (native_metric) {
     const char* id = metric_id + kNativeMetricIdPrefixLen;
     return HandleNativeMetric(isolate, js, id);
   }
@@ skipping 65 matching lines @@
         PrintInvalidParamError(js, "step");
         return true;
       }
     }
     isolate->Resume();
     JSONObject jsobj(js);
     jsobj.AddProperty("type", "Success");
     return true;
   }
 
-  PrintError(js, "VM was not paused");
+  js->PrintError(kVMMustBePaused, NULL);
   return true;
 }
 
 
 static const MethodParameter* pause_params[] = {
   ISOLATE_PARAMETER,
   NULL,
 };
 
 
@@ skipping 275 matching lines @@
   SourceBreakpoint* bpt = LookupBreakpoint(isolate, id);
   if (bpt != NULL) {
     bpt->PrintJSON(js);
     return true;
   }
 
   if (PrintMessage(js, isolate, id)) {
     return true;
   }
 
-  PrintError(js, "Unrecognized object id: %s\n", id);
+  PrintInvalidParamError(js, "objectId");
   return true;
 }
 
 
 static const MethodParameter* get_class_list_params[] = {
   ISOLATE_PARAMETER,
   NULL,
 };
 
 
@@ skipping 237 matching lines @@
     ServiceMethodDescriptor& method = service_methods_[i];
     if (strcmp(method_name, method.name) == 0) {
       return &method;
     }
   }
   return NULL;
 }
 
 
 }  // namespace dart