runat-1128988: Pass Errors through protocol.js correctly

In Output.toJson we were assuming that `JSON.stringify(new Error('x'))`
would give us something useful. It actually just gives `{}`, so we
manually extract the properties of a standard Error.

We were using `error:true` to denote an error, but this was conflicting
with a special property in protocol.js, so we're using isError instead.
(Commands that fail are considered non-exceptional. Exceptions in GCLI
are for when GCLI itself fails)

The converters now need to cope with errors that are objects that look
like Errors (but not actually errors).

In the process of debugging this I refactored the code that kicks off
a remote execution in system.js.

Signed-off-by: Joe Walker <[email protected]>

lib/gcli/cli.js

@@ -2164,11 +2164,23 @@ Output.prototype.convert = function(type, conversionContext) {
 };
 
 Output.prototype.toJson = function() {
+  // Exceptions don't stringify, so we try a bit harder
+  var data = this.data;
+  if (this.error && JSON.stringify(this.data) === '{}') {
+    data = {
+      columnNumber: data.columnNumber,
+      fileName: data.fileName,
+      lineNumber: data.lineNumber,
+      message: data.message,
+      stack: data.stack
+    };
+  }
+
   return {
     typed: this.typed,
     type: this.type,
-    data: this.data,
-    error: this.error
+    data: data,
+    isError: this.error
   };
 };
 

lib/gcli/converters/converters.js

@@ -99,7 +99,7 @@ var errorDomConverter = {
   exec: function(ex, conversionContext) {
     var node = util.createElement(conversionContext.document, 'p');
     node.className = 'gcli-error';
-    node.textContent = ex;
+    node.textContent = errorStringConverter.exec(ex, conversionContext);
     return node;
   }
 };
@@ -112,6 +112,15 @@ var errorStringConverter = {
   from: 'error',
   to: 'string',
   exec: function(ex, conversionContext) {
+    if (typeof ex === 'string') {
+      return ex;
+    }
+    if (ex instanceof Error) {
+      return '' + ex;
+    }
+    if (typeof ex.message === 'string') {
+      return ex.message;
+    }
     return '' + ex;
   }
 };

lib/gcli/system.js

@@ -308,15 +308,9 @@ function addLocalFunctions(specs, front) {
     if (!commandSpec.isParent) {
       commandSpec.exec = function(args, context) {
         var typed = (context.prefix ? context.prefix + ' ' : '') + context.typed;
-
         return front.execute(typed).then(function(reply) {
           var typedData = context.typedData(reply.type, reply.data);
-          if (!reply.error) {
-            return typedData;
-          }
-          else {
-            throw typedData;
-          }
+          return reply.isError ? Promise.reject(typedData) : typedData;
         });
       };
     }