Rework error handling in the service protocol and in Observatory.

runtime/observatory/lib/src/service/object.dart

 // Copyright (c) 2014, 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.
 
 part of service;
 
+/// An RpcException represents an exceptional event that happened
+/// while invoking an rpc.
+abstract class RpcException implements Exception {
+  RpcException(this.message);
+
+  String message;
+}
+
+/// A ServerRpcException represents an error returned by the VM.
+class ServerRpcException extends RpcException {
+  /// A list of well-known server error codes.
+  static const kParseError     = -32700;
+  static const kInvalidRequest = -32600;
+  static const kMethodNotFound = -32601;
+  static const kInvalidParams  = -32602;
+  static const kInternalError  = -32603;
+  static const kVMMustBePaused    = 100;
+  static const kNoBreakAtLine     = 101;
+  static const kNoBreakAtFunction = 102;
+  static const kProfilingDisabled = 200;
+
+  int code;
+  Map data;
+
+  static _getMessage(Map errorMap) {
+    Map data = errorMap['data'];
+    if (data != null && data['details'] != null) {
+      return data['details'];
+    } else {
+      return errorMap['message'];
+    }
+  }
+
+  ServerRpcException.fromMap(Map errorMap) : super(_getMessage(errorMap)) {
+    code = errorMap['code'];
+    data = errorMap['data'];
+  }
+
+  String toString() => 'ServerRpcException(${message})';
+}
+
+/// A NetworkRpcException is used to indicate that an rpc has
+/// been canceled due to network error.
+class NetworkRpcException extends RpcException {
+  NetworkRpcException(String message) : super(message);
+
+  String toString() => 'NetworkRpcException(${message})';
+}
+
+class MalformedResponseRpcException extends RpcException {
+  MalformedResponseRpcException(String message, this.response)
+      : super(message);
+
+  Map response;
+
+  String toString() => 'MalformedResponseRpcException(${message})';
+}
+
+class FakeVMRpcException extends RpcException {
+  FakeVMRpcException(String message) : super(message);
+
+  String toString() => 'FakeVMRpcException(${message})';
+}
+
 /// A [ServiceObject] represents a persistent object within the vm.
 abstract class ServiceObject extends Observable {
   static int LexicalSortName(ServiceObject o1, ServiceObject o2) {
     return o1.name.compareTo(o2.name);
   }
 
   List removeDuplicatesAndSortLexical(List<ServiceObject> list) {
     return list.toSet().toList()..sort(LexicalSortName);
   }
 
@@ skipping 146 matching lines @@
             obj = new PcDescriptors._empty(owner);
             break;
           case 'LocalVarDescriptors':
             obj = new LocalVarDescriptors._empty(owner);
             break;
           case 'TokenStream':
             obj = new TokenStream._empty(owner);
             break;
         }
         break;
-      case 'ServiceError':
-        obj = new ServiceError._empty(owner);
-        break;
       case 'ServiceEvent':
         obj = new ServiceEvent._empty(owner);
         break;
-      case 'ServiceException':
-        obj = new ServiceException._empty(owner);
-        break;
       case 'Script':
         obj = new Script._empty(owner);
         break;
       case 'Socket':
         obj = new Socket._empty(owner);
         break;
       default:
         if (_isInstanceType(type) ||
             type == 'Sentinel') {  // TODO(rmacnak): Separate this out.
           obj = new Instance._empty(owner);
@@ skipping 20 matching lines @@
   Future<ServiceObject> _inProgressReload;
 
   Future<ObservableMap> _fetchDirect() {
     Map params = {
       'objectId': id,
     };
     return isolate.invokeRpcNoUpgrade('getObject', params);
   }
 
   /// Reload [this]. Returns a future which completes to [this] or
-  /// a [ServiceError].
+  /// an exception.
   Future<ServiceObject> reload() {
-    if (id == '') {
-      // Errors don't have ids.
-      assert(type == 'Error');
+    // TODO(turnidge): Checking for a null id should be part of the
+    // "immmutable" check.
+    if (id == null || id == '') {
       return new Future.value(this);
     }
     if (loaded && immutable) {
       return new Future.value(this);
     }
     if (_inProgressReload == null) {
-      _inProgressReload = _fetchDirect().then((ObservableMap map) {
-          var mapType = _stripRef(map['type']);
-          if (mapType != _type) {
-            // If the type changes, return a new object instead of
-            // updating the existing one.
-            //
-            // TODO(turnidge): Check for vmType changing as well?
-            assert(mapType == 'Error' || mapType == 'Sentinel');
-            return new ServiceObject._fromMap(owner, map);
-          }
+      var completer = new Completer<ServiceObject>();
+      _inProgressReload = completer.future;
+      _fetchDirect().then((ObservableMap map) {
+        var mapType = _stripRef(map['type']);
+        if (mapType == 'Sentinel') {
+          // An object may have been collected, etc.
+          completer.complete(new ServiceObject._fromMap(owner, map));
+        } else {
+          // TODO(turnidge): Check for vmType changing as well?
+          assert(mapType == _type);
           update(map);
-          return this;
+          completer.complete(this);
+        }
+
+      }).catchError((e, st) {
+        Logger.root.severe("Unable to reload object: $e\n$st");
+        _inProgressReload = null;
+        completer.completeError(e, st);
       }).whenComplete(() {
-          // This reload is complete.
-          _inProgressReload = null;
+        // This reload is complete.
+        _inProgressReload = null;
       });
     }
     return _inProgressReload;
   }
 
   /// Update [this] using [map] as a source. [map] can be a reference.
   void update(ObservableMap map) {
     assert(_isServiceMap(map));
 
     // Don't allow the type to change on an object update.
-    // TODO(turnidge): Make this a ServiceError?
     var mapIsRef = _hasRef(map['type']);
     var mapType = _stripRef(map['type']);
     assert(_type == null || _type == mapType);
 
     if (_id != null && _id != map['id']) {
       // It is only safe to change an id when the object isn't cacheable.
       assert(!canCache);
     }
     _id = map['id'];
 
@@ skipping 62 matching lines @@
   /// The result may come from the cache.  The result will not necessarily
   /// be [loaded].
   ServiceObject getFromMap(ObservableMap map);
 }
 
 /// State for a VM being inspected.
 abstract class VM extends ServiceObjectOwner {
   @reflectable VM get vm => this;
   @reflectable Isolate get isolate => null;
 
+  // TODO(turnidge): The connection should not be stored in the VM object.
+  bool get isDisconnected;
+
   // TODO(johnmccutchan): Ensure that isolates do not end up in _cache.
   Map<String,ServiceObject> _cache = new Map<String,ServiceObject>();
   final ObservableMap<String,Isolate> _isolateCache =
       new ObservableMap<String,Isolate>();
 
   @reflectable Iterable<Isolate> get isolates => _isolateCache.values;
 
   @observable String version = 'unknown';
   @observable String targetCPU;
   @observable int architectureBits;
   @observable bool assertsEnabled = false;
   @observable bool typeChecksEnabled = false;
   @observable String pid = '';
   @observable DateTime startTime;
   @observable DateTime refreshTime;
   @observable Duration get upTime =>
       (new DateTime.now().difference(startTime));
 
   VM() : super._empty(null) {
     name = 'vm';
     vmName = 'vm';
     _cache['vm'] = this;
     update(toObservable({'id':'vm', 'type':'@VM'}));
   }
 
-  final StreamController<ServiceException> exceptions =
-      new StreamController.broadcast();
-  final StreamController<ServiceError> errors =
-      new StreamController.broadcast();
   final StreamController<ServiceEvent> events =
       new StreamController.broadcast();
 
   void postServiceEvent(Map response, ByteData data) {
     var map = toObservable(response);
     assert(!map.containsKey('_data'));
     if (data != null) {
       map['_data'] = data;
     }
     if (map['type'] != 'ServiceEvent') {
@@ skipping 73 matching lines @@
   // Note that this function does not reload the isolate if it found
   // in the cache.
   Future<ServiceObject> getIsolate(String isolateId) {
     if (!loaded) {
       // Trigger a VM load, then get the isolate.
       return load().then((_) => getIsolate(isolateId)).catchError(_ignoreError);
     }
     return new Future.value(_isolateCache[isolateId]);
   }
 
-  Future<ObservableMap> _processMap(ObservableMap map) {
-    // Verify that the top level response is a service map.
-    if (!_isServiceMap(map)) {
-      return new Future.error(
-            new ServiceObject._fromMap(this, toObservable({
-        'type': 'ServiceException',
-        'kind': 'ResponseFormatException',
-        'response': map,
-        'message': "Response is missing the 'type' field.",
-      })));
-    }
-    // Preemptively capture ServiceError and ServiceExceptions.
-    if (map['type'] == 'ServiceError') {
-      return new Future.error(new ServiceObject._fromMap(this, map));
-    } else if (map['type'] == 'ServiceException') {
-      return new Future.error(new ServiceObject._fromMap(this, map));
-    }
-    // map is now guaranteed to be a non-error/exception ServiceObject.
-    return new Future.value(map);
-  }
-
   // Implemented in subclass.
   Future<Map> invokeRpcRaw(String method, Map params);
 
   Future<ObservableMap> invokeRpcNoUpgrade(String method, Map params) {
     return invokeRpcRaw(method, params).then((Map response) {
       var map = toObservable(response);
       if (Tracer.current != null) {
         Tracer.current.trace("Received response for ${method}/${params}}",
                              map:map);
       }
-
-      // Check for ill-formed responses.
-      return _processMap(map);
-    }).catchError((error) {
-
-      // ServiceError, forward to VM's ServiceError stream.
-      errors.add(error);
-      return new Future.error(error);
-    }, test: (e) => e is ServiceError).catchError((exception) {
-
-      // ServiceException, forward to VM's ServiceException stream.
-      exceptions.add(exception);
-      return new Future.error(exception);
-    }, test: (e) => e is ServiceException);
+      if (!_isServiceMap(map)) {
+        var exception =
+            new MalformedResponseRpcException(
+                "Response is missing the 'type' field", map);
+        return new Future.error(exception);
+      }
+      return new Future.value(map);
+    }).catchError((e) {
+      // Errors pass through.
+      return new Future.error(e);
+    });
   }
 
   Future<ServiceObject> invokeRpc(String method, Map params) {
     return invokeRpcNoUpgrade(method, params).then((ObservableMap response) {
       var obj = new ServiceObject._fromMap(this, response);
       if ((obj != null) && obj.canCache) {
         String objId = obj.id;
         _cache.putIfAbsent(objId, () => obj);
       }
       return obj;
     });
   }
 
   Future<ObservableMap> _fetchDirect() {
+    print("FETCH DIRECT VM");
     return invokeRpcNoUpgrade('getVM', {});
   }
 
   Future<ServiceObject> getFlagList() {
     return invokeRpc('getFlagList', {});
   }
 
   /// Force the VM to disconnect.
   void disconnect();
   /// Completes when the VM first connects.
@@ skipping 78 matching lines @@
   Future get onConnect {
     if (_onConnect != null) {
       return _onConnect;
     }
     _onConnect = new Future.value(this);
     return _onConnect;
   }
   // Only complete when requested.
   Completer _onDisconnect = new Completer();
   Future get onDisconnect => _onDisconnect.future;
+  bool get isDisconnected => _onDisconnect.isCompleted;
 
   Future<Map> invokeRpcRaw(String method, Map params) {
     if (params.isEmpty) {
       params = null;
     }
     var key = _canonicalizeUri(new Uri(path: method, queryParameters: params));
     var response = _responses[key];
     if (response == null) {
-      return new Future.error({
-        'type': 'ServiceException',
-        'kind': 'NotContainedInResponses',
-        'key': key
-      });
+      return new Future.error(new FakeVMRpcException(
+          "Unable to find key '${key}' in cached response set"));
     }
     return new Future.value(response);
   }
 }
 
 
 /// Snapshot in time of tag counters.
 class TagProfileSnapshot {
   final double seconds;
   final List<int> counters;
@@ skipping 456 matching lines @@
         // Ignore GC events for now.
         break;
 
       default:
         // Log unrecognized events.
         Logger.root.severe('Unrecognized event: $event');
         break;
     }
   }
 
-  Future<ServiceObject> addBreakpoint(Script script, int line) {
+  Future<ServiceObject> addBreakpoint(Script script, int line) async {
     // TODO(turnidge): Pass line as an int instead of a string.
-    Map params = {
-      'scriptId': script.id,
-      'line': '$line',
-    };
-    return invokeRpc('addBreakpoint', params).then((result) {
-      if (result is DartError) {
-        return result;
-      }
-      Breakpoint bpt = result;
+    try {
+      Map params = {
+        'scriptId': script.id,
+        'line': '$line',
+      };
+      Breakpoint bpt = await invokeRpc('addBreakpoint', params);
       if (bpt.resolved &&
           script.loaded &&
-          script.tokenToLine(result.tokenPos) != line) {
-        // Unable to set a breakpoint at desired line.
-        script.lines[line - 1].possibleBpt = false;
+          script.tokenToLine(bpt.tokenPos) != line) {
+        // TODO(turnidge): Can this still happen?
+        script.getLine(line).possibleBpt = false;
       }
-      return result;
-    });
+      return bpt;
+    } on ServerRpcException catch(e) {
+      if (e.code == ServerRpcException.kNoBreakAtLine) {
+        // Unable to set a breakpoint at the desired line.
+        script.getLine(line).possibleBpt = false;
+      }
+      rethrow;
+    }
   }
 
   Future<ServiceObject> addBreakpointAtEntry(ServiceFunction function) {
     return invokeRpc('addBreakpointAtEntry',
                      { 'functionId': function.id });
   }
 
   Future removeBreakpoint(Breakpoint bpt) {
     return invokeRpc('removeBreakpoint',
                      { 'breakpointId': bpt.id });
   }
 
   Future pause() {
-    return invokeRpc('pause', {}).then((result) {
-        if (result is DartError) {
-          // TODO(turnidge): Handle this more gracefully.
-          Logger.root.severe(result.message);
-        }
-        return result;
-      });
+    return invokeRpc('pause', {});
   }
 
   Future resume() {
-    return invokeRpc('resume', {}).then((result) {
-        if (result is DartError) {
-          // TODO(turnidge): Handle this more gracefully.
-          Logger.root.severe(result.message);
-        }
-        return result;
-      });
+    return invokeRpc('resume', {});
   }
 
   Future stepInto() {
-    return invokeRpc('resume', {'step': 'into'}).then((result) {
-        if (result is DartError) {
-          // TODO(turnidge): Handle this more gracefully.
-          Logger.root.severe(result.message);
-        }
-        return result;
-      });
+    return invokeRpc('resume', {'step': 'into'});
   }
 
   Future stepOver() {
-    return invokeRpc('resume', {'step': 'over'}).then((result) {
-        if (result is DartError) {
-          // TODO(turnidge): Handle this more gracefully.
-          Logger.root.severe(result.message);
-        }
-        return result;
-      });
+    return invokeRpc('resume', {'step': 'over'});
   }
 
   Future stepOut() {
-    return invokeRpc('resume', {'step': 'out'}).then((result) {
-        if (result is DartError) {
-          // TODO(turnidge): Handle this more gracefully.
-          Logger.root.severe(result.message);
-        }
-        return result;
-      });
+    return invokeRpc('resume', {'step': 'out'});
   }
 
   Future setName(String newName) {
-    Map params = {
-      'name': newName,
-    };
-    return invokeRpc('setName', params);
+    return invokeRpc('setName', {'name': newName});
   }
 
   Future<ServiceMap> getStack() {
-    return invokeRpc('getStack', {}).then((result) {
-        if (result is DartError) {
-          // TODO(turnidge): Handle this more gracefully.
-          Logger.root.severe(result.message);
-        }
-        return result;
-      });
+    return invokeRpc('getStack', {});
   }
 
   Future<ServiceObject> eval(ServiceObject target,
                              String expression) {
     Map params = {
       'targetId': target.id,
       'expression': expression,
     };
     return invokeRpc('eval', params);
   }
@@ skipping 57 matching lines @@
       new ObservableMap<String, ServiceMetric>();
 
   final ObservableMap<String, ServiceMetric> nativeMetrics =
       new ObservableMap<String, ServiceMetric>();
 
   Future<ObservableMap<String, ServiceMetric>> _refreshMetrics(
       String metricType,
       ObservableMap<String, ServiceMetric> metricsMap) {
     return invokeRpc('getIsolateMetricList',
                      { 'type': metricType }).then((result) {
-      if (result is DartError) {
-        // TODO(turnidge): Handle this more gracefully.
-        Logger.root.severe(result.message);
-        return null;
-      }
       // Clear metrics map.
       metricsMap.clear();
       // Repopulate metrics map.
       var metrics = result['metrics'];
       for (var metric in metrics) {
         metricsMap[metric.id] = metric;
       }
       return metricsMap;
     });
   }
@@ skipping 90 matching lines @@
     message = map['message'];
     exception = new ServiceObject._fromMap(owner, map['exception']);
     stacktrace = new ServiceObject._fromMap(owner, map['stacktrace']);
     name = 'DartError $kind';
     vmName = name;
   }
 
   String toString() => 'DartError($message)';
 }
 
-/// A [ServiceError] is an error that was triggered in the service
-/// server or client. Errors are prorammer mistakes that could have
-/// been prevented, for example, requesting a non-existant path over the
-/// service.
-class ServiceError extends ServiceObject {
-  ServiceError._empty(ServiceObjectOwner owner) : super._empty(owner);
-
-  @observable String kind;
-  @observable String message;
-
-  void _update(ObservableMap map, bool mapIsRef) {
-    _loaded = true;
-    kind = map['kind'];
-    message = map['message'];
-    name = 'ServiceError $kind';
-    vmName = name;
-  }
-
-  String toString() => 'ServiceError($message)';
-}
-
-/// A [ServiceException] is an exception that was triggered in the service
-/// server or client. Exceptions are events that should be handled,
-/// for example, an isolate went away or the connection to the VM was lost.
-class ServiceException extends ServiceObject {
-  ServiceException._empty(ServiceObject owner) : super._empty(owner);
-
-  @observable String kind;
-  @observable String message;
-  @observable dynamic response;
-
-  void _update(ObservableMap map, bool mapIsRef) {
-    kind = map['kind'];
-    message = map['message'];
-    response = map['response'];
-    name = 'ServiceException $kind';
-    vmName = name;
-  }
-
-  String toString() => 'ServiceException($message)';
-}
-
 /// A [ServiceEvent] is an asynchronous event notification from the vm.
 class ServiceEvent extends ServiceObject {
   /// The possible 'eventType' values.
   static const kIsolateStart       = 'IsolateStart';
   static const kIsolateExit        = 'IsolateExit';
   static const kIsolateUpdate      = 'IsolateUpdate';
   static const kPauseStart         = 'PauseStart';
   static const kPauseExit          = 'PauseExit';
   static const kPauseBreakpoint    = 'PauseBreakpoint';
   static const kPauseInterrupted   = 'PauseInterrupted';
@@ skipping 15 matching lines @@
 
   @observable String eventType;
   @observable Breakpoint breakpoint;
   @observable ServiceMap topFrame;
   @observable ServiceMap exception;
   @observable ServiceObject inspectee;
   @observable ByteData data;
   @observable int count;
   @observable String reason;
 
+  @observable bool get isPauseEvent {
+    return (eventType == kPauseStart ||
+            eventType == kPauseExit ||
+            eventType == kPauseBreakpoint ||
+            eventType == kPauseInterrupted ||
+            eventType == kPauseException);
+  }
+
   void _update(ObservableMap map, bool mapIsRef) {
     _loaded = true;
     _upgradeCollection(map, owner);
     assert(map['isolate'] == null || owner == map['isolate']);
     eventType = map['eventType'];
+    notifyPropertyChange(#isPauseEvent, 0, 1);
     name = 'ServiceEvent $eventType';
     vmName = name;
     if (map['breakpoint'] != null) {
       breakpoint = map['breakpoint'];
     }
     if (map['topFrame'] != null) {
       topFrame = map['topFrame'];
     }
     if (map['exception'] != null) {
       exception = map['exception'];
@@ skipping 184 matching lines @@
   @observable int tokenPos;
   @observable int endTokenPos;
 
   @observable ServiceMap error;
   @observable int vmCid;
 
   final Allocations newSpace = new Allocations();
   final Allocations oldSpace = new Allocations();
   final AllocationCount promotedByLastNewGC = new AllocationCount();
 
-  bool get hasNoAllocations => newSpace.empty && oldSpace.empty;
+  @observable bool get hasNoAllocations => newSpace.empty && oldSpace.empty;
 
   @reflectable final fields = new ObservableList<Field>();
   @reflectable final functions = new ObservableList<ServiceFunction>();
 
   @observable Class superclass;
   @reflectable final interfaces = new ObservableList<Instance>();
   @reflectable final subclasses = new ObservableList<Class>();
 
   bool get canCache => true;
   bool get immutable => false;
@@ skipping 55 matching lines @@
     // Work-around Object not tracking its subclasses in the VM.
     if (superclass != null && superclass.name == "Object") {
       superclass._addSubclass(this);
     }
     error = map['error'];
 
     var allocationStats = map['allocationStats'];
     if (allocationStats != null) {
       newSpace.update(allocationStats['new']);
       oldSpace.update(allocationStats['old']);
+      notifyPropertyChange(#hasNoAllocations, 0, 1);
       promotedByLastNewGC.instances = allocationStats['promotedInstances'];
       promotedByLastNewGC.bytes = allocationStats['promotedBytes'];
     }
   }
 
   void _addSubclass(Class subclass) {
     if (subclasses.contains(subclass)) {
       return;
     }
     subclasses.add(subclass);
@@ skipping 436 matching lines @@
   @observable Library library;
   bool get canCache => true;
   bool get immutable => true;
 
   String _shortUrl;
   String _url;
 
   Script._empty(ServiceObjectOwner owner) : super._empty(owner);
 
   ScriptLine getLine(int line) {
+    assert(_loaded);
     assert(line >= 1);
     return lines[line - lineOffset - 1];
   }
 
   /// This function maps a token position to a line number.
   int tokenToLine(int token) => _tokenToLine[token];
   Map _tokenToLine = {};
 
   /// This function maps a token position to a column number.
   int tokenToCol(int token) => _tokenToCol[token];
@@ skipping 440 matching lines @@
       return Native;
     } else if (s == 'Dart') {
       return Dart;
     } else if (s == 'Collected') {
       return Collected;
     } else if (s == 'Tag') {
       return Tag;
     } else if (s == 'Stub') {
       return Stub;
     }
-    Logger.root.severe('Unrecognized code kind: $s');
+    Logger.root.severe("Unrecognized code kind: '$s'");
     throw new FallThroughError();
   }
   static const Collected = const CodeKind._internal('Collected');
   static const Dart = const CodeKind._internal('Dart');
   static const Native = const CodeKind._internal('Native');
   static const Stub = const CodeKind._internal('Stub');
   static const Tag = const CodeKind._internal('Tag');
 }
 
 class CodeInlineInterval {
@@ skipping 53 matching lines @@
         }
         // Load the script and then update descriptors.
         script.load().then(_updateDescriptors);
       });
       return;
     }
     // Load the script and then update descriptors.
     function.script.load().then(_updateDescriptors);
   }
 
-  /// Reload [this]. Returns a future which completes to [this] or
-  /// a [ServiceError].
+  /// Reload [this]. Returns a future which completes to [this] or an
+  /// exception.
   Future<ServiceObject> reload() {
     assert(kind != null);
     if (isDartCode) {
       // We only reload Dart code.
       return super.reload();
     }
     return new Future.value(this);
   }
 
   void _update(ObservableMap m, bool mapIsRef) {
@@ skipping 372 matching lines @@
     var v = list[i];
     if ((v is ObservableMap) && _isServiceMap(v)) {
       list[i] = owner.getFromMap(v);
     } else if (v is ObservableList) {
       _upgradeObservableList(v, owner);
     } else if (v is ObservableMap) {
       _upgradeObservableMap(v, owner);
     }
   }
 }