node-triton#98 triton inst get ID fails obtusely on a destroyed instance

CHANGES.md

@@ -2,6 +2,9 @@
 
 ## 4.6.0 (not yet released)
 
+- #98 `triton inst get ID` for a deleted instance will now emit the instance
+  object and error less obtusely. This adds a new `InstanceDeleted` error code
+  from `TritonApi`.
 - PUBAPI-1233 firewalls: `triton fwrule ...`
 - PUBAPI-1234 instance snapshots: `triton inst snapshot ...`
 - #52 Fix 'triton ssh ...' stdout/stderr to fully flush with node >= 4.x.

lib/cli.js

@@ -235,8 +235,8 @@ function CLI() {
             '    0   Successful completion.',
             '    1   An error occurred.',
             '    2   Usage error.',
-            '    3   "ResourceNotFound" error. Returned when an instance, image,',
+            '    3   "ResourceNotFound" error (when an instance, image, etc. with',
-            '        package, etc. with the given name or id is not found.'
+            '        the given name or id is not found) or "InstanceDeleted" error.'
             /* END JSSTYLED */
         ].join('\n')
     });

lib/do_instance/do_get.js

@@ -5,7 +5,7 @@
  */
 
 /*
- * Copyright 2015 Joyent, Inc.
+ * Copyright 2016 Joyent, Inc.
  *
  * `triton instance get ...`
  */
@@ -20,16 +20,14 @@ function do_get(subcmd, opts, args, cb) {
     }
 
     this.top.tritonapi.getInstance(args[0], function (err, inst) {
-        if (err) {
+        if (inst) {
-            return cb(err);
-        }
-
             if (opts.json) {
                 console.log(JSON.stringify(inst));
             } else {
                 console.log(JSON.stringify(inst, null, 4));
             }
-        cb();
+        }
+        cb(err);
     });
 }
 
@@ -54,7 +52,10 @@ do_get.help = (
     + '\n'
     + '{{options}}'
     + '\n'
-    + 'Note: Currently this dumps prettified JSON by default. That might change\n'
+    + 'A *deleted* instance may still respond with the instance object. In that\n'
+    + 'case a the instance will be print *and* an error will be raised.\n'
+    + '\n'
+    + 'Currently this dumps prettified JSON by default. That might change\n'
     + 'in the future. Use "-j" to explicitly get JSON output.\n'
     /* END JSSTYLED */
 );

lib/errors.js

@@ -5,7 +5,7 @@
  */
 
 /*
- * Copyright 2015 Joyent, Inc.
+ * Copyright 2016 Joyent, Inc.
  *
  * Error classes that the joyent CLI may produce.
  */
@@ -233,6 +233,24 @@ function ResourceNotFoundError(cause, msg) {
 util.inherits(ResourceNotFoundError, _TritonBaseWError);
 
 
+/**
+ * An instance was deleted.
+ */
+function InstanceDeletedError(cause, msg) {
+    if (msg === undefined) {
+        msg = cause;
+        cause = undefined;
+    }
+    _TritonBaseWError.call(this, {
+        cause: cause,
+        message: msg,
+        code: 'InstanceDeleted',
+        exitStatus: 3
+    });
+}
+util.inherits(InstanceDeletedError, _TritonBaseWError);
+
+
 /**
  * Multiple errors in a group.
  */
@@ -266,6 +284,7 @@ module.exports = {
     SelfSignedCertError: SelfSignedCertError,
     TimeoutError: TimeoutError,
     ResourceNotFoundError: ResourceNotFoundError,
+    InstanceDeletedError: InstanceDeletedError,
     MultiError: MultiError
 };
 // vim: set softtabstop=4 shiftwidth=4:

lib/tritonapi.js

@@ -5,7 +5,7 @@
  */
 
 /*
- * Copyright 2015 Joyent, Inc.
+ * Copyright 2016 Joyent, Inc.
  *
  * Core TritonApi client driver class.
  */
@@ -602,17 +602,18 @@ TritonApi.prototype.getNetwork = function getNetwork(name, cb) {
 /**
  * Get an instance.
  *
- * Alternative call signature: `getInstance(id, callback)`.
+ * Alternative call signature: `getInstance(id, cb)`.
  *
  * @param {Object} opts
  *      - {UUID} id: The instance ID, name, or short ID. Required.
  *      - {Array} fields: Optional. An array of instance field names that are
  *        wanted by the caller. This *can* allow the implementation to avoid
  *        extra API calls. E.g. `['id', 'name']`.
- * @param {Function} callback `function (err, inst, res)`
+ * @param {Function} cb `function (err, inst, res)`
- *      On success, `res` is the response object from a `GetMachine`, if one
+ *      Note that deleted instances will result in `err` being a
- *      was made (possibly not if the instance was retrieved from `ListMachines`
+ *      `InstanceDeletedError` and `inst` being defined. On success, `res` is
- *      calls).
+ *      the response object from a `GetMachine`, if one was made (possibly not
+ *      if the instance was retrieved from `ListMachines` calls).
  */
 TritonApi.prototype.getInstance = function getInstance(opts, cb) {
     var self = this;
@@ -624,6 +625,24 @@ TritonApi.prototype.getInstance = function getInstance(opts, cb) {
     assert.optionalArrayOfString(opts.fields, 'opts.fields');
     assert.func(cb, 'cb');
 
+    /*
+     * Some wrapping/massaging of some CloudAPI GetMachine errors.
+     */
+    var errFromGetMachineErr = function (err) {
+        if (!err) {
+            // jsl:pass
+        } else if (err.restCode === 'ResourceNotFound') {
+            // The CloudApi 404 error message sucks: "VM not found".
+            err = new errors.ResourceNotFoundError(err,
+                format('instance with id %s was not found', opts.id));
+        } else if (err.statusCode === 410) {
+            // GetMachine returns '410 Gone' for deleted machines.
+            err = new errors.InstanceDeletedError(err,
+                format('instance %s was deleted', opts.id));
+        }
+        return err;
+    };
+
     var res;
     var shortId;
     var inst;
@@ -646,11 +665,7 @@ TritonApi.prototype.getInstance = function getInstance(opts, cb) {
             self.cloudapi.getMachine(uuid, function (err, inst_, res_) {
                 res = res_;
                 inst = inst_;
-                if (err && err.restCode === 'ResourceNotFound') {
+                err = errFromGetMachineErr(err);
-                    // The CloudApi 404 error message sucks: "VM not found".
-                    err = new errors.ResourceNotFoundError(err,
-                        format('instance with id %s was not found', opts.id));
-                }
                 next(err);
             });
         },
@@ -739,19 +754,13 @@ TritonApi.prototype.getInstance = function getInstance(opts, cb) {
             self.cloudapi.getMachine(uuid, function (err, inst_, res_) {
                 res = res_;
                 inst = inst_;
-                if (err && err.restCode === 'ResourceNotFound') {
+                err = errFromGetMachineErr(err);
-                    // The CloudApi 404 error message sucks: "VM not found".
-                    err = new errors.ResourceNotFoundError(err,
-                        format('instance with id %s was not found', opts.id));
-                }
                 next(err);
             });
         }
     ]}, function (err) {
-        if (err) {
+        if (err || inst) {
-            cb(err);
+            cb(err, inst, res);
-        } else if (inst) {
-            cb(null, inst, res);
         } else {
             cb(new errors.ResourceNotFoundError(format(
                 'no instance with name or short id "%s" was found', opts.id)));

test/integration/cli-manage-workflow.test.js

@@ -140,6 +140,21 @@ test('triton manage workflow', opts, function (tt) {
         });
     });
 
+    // Test the '410 Gone' handling from CloudAPI GetMachine.
+    tt.test('  triton inst get (deleted)', function (t) {
+        h.triton(['inst', 'get', instance.id], function (err, stdout, stderr) {
+            t.ok(err, 'got err: ' + err);
+            t.equal(err.code, 3, 'exit status of 3');
+            var errCodeRe = /InstanceDeleted/;
+            t.ok(errCodeRe.exec(stderr),
+                f('stderr matched %s: %j', errCodeRe, stderr));
+            t.ok(stdout, 'still got stdout');
+            var inst = JSON.parse(stdout);
+            t.equal(inst.state, 'deleted', 'instance state is "deleted"');
+            t.end();
+        });
+    });
+
     // TODO: would be nice to have a `triton ssh cat /var/log/boot.log` to
     //      verify the user-script worked.