common: promisify: deal with trailing undefined (googleapis#2130)

Author: ofrobots

packages/common/src/util.js

@@ -706,14 +706,18 @@ function promisify(originalMethod, options) {
   var slice = Array.prototype.slice;
 
   var wrapper = function() {
-    var args = slice.call(arguments);
-    var hasCallback = is.fn(args[args.length - 1]);
-    var context = this;
-
-    if (hasCallback) {
-      return originalMethod.apply(context, args);
+    var last;
+    for (last = arguments.length - 1; last >= 0; last--) {
+      var arg = arguments[last];
+      if (arg === undefined) continue;  // skip trailing undefined.
+      if (!is.fn(arg)) break;           // non-callback last argument found.
+      return originalMethod.apply(this, arguments);
     }
 
+    // peel trailing undefined.
+    var args = slice.call(arguments, 0, last + 1);
+    var context = this;
+
     var PromiseCtor = Promise;
 
     // Because dedupe will likely create a single install of

packages/common/test/util.js

@@ -1648,6 +1648,32 @@ describe('common/util', function() {
 
       assert.strictEqual(FakeClass.prototype.methodName, method);
     });
+
+    describe('trailing undefined arguments', function() {
+      it('should not return a promise in callback mode', function(done) {
+        var func = util.promisify(function(optional, callback) {
+          assert(is.fn(optional));
+          optional(null);
+        });
+
+        var returnVal = func(function() {
+          assert(!returnVal);
+          done();
+        });
+      });
+
+      it('should return a promise when callback omitted', function(done) {
+        var func = util.promisify(function(optional, callback) {
+          assert.strictEqual(arguments.length, 1);
+          assert(is.fn(optional));
+          optional(null);
+        });
+
+        var returnVal = func(undefined, undefined).then(function() {
+          done();
+        });
+      });
+    });
   });
 
   describe('promisify', function() {