Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(299)

Unified Diff: runtime/vm/service.cc

Issue 1120133002: Rework error handling in the service protocol and in Observatory. (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
Patch Set: fix tests Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « runtime/vm/profiler_service.cc ('k') | runtime/vm/service_test.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: runtime/vm/service.cc
diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc
index ace6b6ac3a4675c2b7105764aac18e88f0359fb5..cf52a439f1b05e72b9af0633f26ec180789c5cab 100644
--- a/runtime/vm/service.cc
+++ b/runtime/vm/service.cc
@@ -79,81 +79,23 @@ static uint8_t* allocator(uint8_t* ptr, intptr_t old_size, intptr_t 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());
-}
-
-
-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);
+ js->PrintError(kMethodNotFound, NULL);
}
@@ -1224,13 +1166,33 @@ static RawObject* LookupHeapObject(Isolate* isolate,
}
-static void PrintSentinel(JSONStream* js,
- const char* id,
- const char* preview) {
+enum SentinelType {
+ kCollectedSentinel,
+ kExpiredSentinel,
+ kFreeSentinel,
+};
+
+
+static void PrintSentinel(JSONStream* js, SentinelType sentinel_type) {
JSONObject jsobj(js);
jsobj.AddProperty("type", "Sentinel");
- jsobj.AddProperty("id", id);
- jsobj.AddProperty("valueAsString", preview);
+ switch (sentinel_type) {
+ case kCollectedSentinel:
+ jsobj.AddProperty("kind", "Collected");
+ jsobj.AddProperty("valueAsString", "<collected>");
+ break;
+ case kExpiredSentinel:
+ jsobj.AddProperty("kind", "Expired");
+ jsobj.AddProperty("valueAsString", "<expired>");
+ break;
+ case kFreeSentinel:
+ jsobj.AddProperty("kind", "Free");
+ jsobj.AddProperty("valueAsString", "<free>");
+ break;
+ default:
+ UNIMPLEMENTED();
+ break;
+ }
}
@@ -1268,9 +1230,10 @@ static bool PrintMessage(JSONStream* js, Isolate* isolate, const char* id) {
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, kExpiredSentinel);
+ return true;
}
SnapshotReader reader(message->data(),
message->len(),
@@ -1280,8 +1243,6 @@ static bool PrintMessage(JSONStream* js, Isolate* isolate, const char* id) {
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;
@@ -1349,7 +1310,7 @@ static bool GetInboundReferences(Isolate* isolate, JSONStream* js) {
return true;
}
const char* limit_cstr = js->LookupParam("limit");
- if (target_id == NULL) {
+ if (limit_cstr == NULL) {
PrintMissingParamError(js, "limit");
return true;
}
@@ -1367,17 +1328,12 @@ static bool GetInboundReferences(Isolate* isolate, JSONStream* js) {
}
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, kCollectedSentinel);
} else if (lookup_result == ObjectIdRing::kExpired) {
- PrintErrorWithKind(
- js, "InboundReferencesExpired",
- "attempt to find a retaining path for an expired object\n");
- return true;
+ PrintSentinel(js, kExpiredSentinel);
+ } else {
+ PrintInvalidParamError(js, "targetId");
}
- PrintInvalidParamError(js, "targetId");
return true;
}
return PrintInboundReferences(isolate, &obj, limit, js);
@@ -1453,7 +1409,7 @@ static bool GetRetainingPath(Isolate* isolate, JSONStream* js) {
return true;
}
const char* limit_cstr = js->LookupParam("limit");
- if (target_id == NULL) {
+ if (limit_cstr == NULL) {
PrintMissingParamError(js, "limit");
return true;
}
@@ -1471,17 +1427,12 @@ static bool GetRetainingPath(Isolate* isolate, JSONStream* js) {
}
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, kCollectedSentinel);
} else if (lookup_result == ObjectIdRing::kExpired) {
- PrintErrorWithKind(
- js, "RetainingPathExpired",
- "attempt to find a retaining path for an expired object\n");
- return true;
+ PrintSentinel(js, kExpiredSentinel);
+ } else {
+ PrintInvalidParamError(js, "targetId");
}
- PrintInvalidParamError(js, "targetId");
return true;
}
return PrintRetainingPath(isolate, &obj, limit, js);
@@ -1505,17 +1456,12 @@ static bool GetRetainedSize(Isolate* isolate, JSONStream* js) {
&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, kCollectedSentinel);
} else if (lookup_result == ObjectIdRing::kExpired) {
- PrintErrorWithKind(
- js, "RetainedExpired",
- "attempt to calculate size retained by an expired object\n");
- return true;
+ PrintSentinel(js, kExpiredSentinel);
+ } else {
+ PrintInvalidParamError(js, "targetId");
}
- PrintInvalidParamError(js, "targetId");
return true;
}
if (obj.IsClass()) {
@@ -1534,9 +1480,10 @@ static bool GetRetainedSize(Isolate* isolate, JSONStream* js) {
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;
}
@@ -1564,9 +1511,9 @@ static bool Eval(Isolate* isolate, JSONStream* js) {
&lookup_result));
if (obj.raw() == Object::sentinel().raw()) {
if (lookup_result == ObjectIdRing::kCollected) {
- PrintSentinel(js, "objects/collected", "<collected>");
+ PrintSentinel(js, kCollectedSentinel);
} else if (lookup_result == ObjectIdRing::kExpired) {
- PrintSentinel(js, "objects/expired", "<expired>");
+ PrintSentinel(js, kExpiredSentinel);
} else {
PrintInvalidParamError(js, "targetId");
}
@@ -1600,9 +1547,10 @@ static bool Eval(Isolate* isolate, JSONStream* js) {
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;
}
@@ -1678,7 +1626,7 @@ static bool GetInstances(Isolate* isolate, JSONStream* js) {
return true;
}
const char* limit_cstr = js->LookupParam("limit");
- if (target_id == NULL) {
+ if (limit_cstr == NULL) {
PrintMissingParamError(js, "limit");
return true;
}
@@ -1809,9 +1757,11 @@ static bool GetHitsOrSites(Isolate* isolate, JSONStream* js, bool as_sites) {
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;
}
@@ -1862,7 +1812,7 @@ static bool AddBreakpoint(Isolate* isolate, JSONStream* js) {
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);
@@ -1888,9 +1838,7 @@ static bool AddBreakpointAtEntry(Isolate* isolate, JSONStream* js) {
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);
@@ -1968,7 +1916,7 @@ static bool HandleNativeMetric(Isolate* isolate,
}
current = current->next();
}
- PrintError(js, "Native Metric %s not found\n", id);
+ PrintInvalidParamError(js, "metricId");
return true;
}
@@ -2015,7 +1963,7 @@ static bool HandleDartMetric(Isolate* isolate, JSONStream* js, const char* id) {
buffer->AddString(String::Cast(result).ToCString());
return true;
}
- PrintError(js, "Dart Metric %s not found\n", id);
+ PrintInvalidParamError(js, "metricId");
return true;
}
@@ -2064,7 +2012,8 @@ static bool GetIsolateMetric(Isolate* isolate, JSONStream* js) {
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/";
@@ -2150,7 +2099,7 @@ static bool Resume(Isolate* isolate, JSONStream* js) {
return true;
}
- PrintError(js, "VM was not paused");
+ js->PrintError(kVMMustBePaused, NULL);
return true;
}
@@ -2376,7 +2325,7 @@ static bool GetObjectByAddress(Isolate* isolate, JSONStream* js) {
object = isolate->heap()->FindObject(&visitor);
}
if (object.IsNull()) {
- PrintSentinel(js, "objects/free", "<free>");
+ PrintSentinel(js, kFreeSentinel);
} else {
object.PrintJSON(js, ref);
}
@@ -2428,10 +2377,10 @@ static bool GetObject(Isolate* isolate, JSONStream* js) {
obj.PrintJSON(js, false);
return true;
} else if (lookup_result == ObjectIdRing::kCollected) {
- PrintSentinel(js, "objects/collected", "<collected>");
+ PrintSentinel(js, kCollectedSentinel);
return true;
} else if (lookup_result == ObjectIdRing::kExpired) {
- PrintSentinel(js, "objects/expired", "<expired>");
+ PrintSentinel(js, kExpiredSentinel);
return true;
}
@@ -2446,7 +2395,7 @@ static bool GetObject(Isolate* isolate, JSONStream* js) {
return true;
}
- PrintError(js, "Unrecognized object id: %s\n", id);
+ PrintInvalidParamError(js, "objectId");
return true;
}
« no previous file with comments | « runtime/vm/profiler_service.cc ('k') | runtime/vm/service_test.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698