From 725ef6b38723307aadb66b80f2f63fd537d8c727 Mon Sep 17 00:00:00 2001
From: Ali Ijaz Sheikh <ofrobots@google.com>
Date: Mon, 27 Mar 2017 13:40:48 -0700
Subject: [PATCH] common: promisify: deal with trailing undefined

---
 packages/common/src/util.js  | 16 ++++++++++------
 packages/common/test/util.js | 26 ++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/packages/common/src/util.js b/packages/common/src/util.js
index d4592cb0319..834e75e94d7 100644
--- a/packages/common/src/util.js
+++ b/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
diff --git a/packages/common/test/util.js b/packages/common/test/util.js
index ad807ca2819..cb758f8dac5 100644
--- a/packages/common/test/util.js
+++ b/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() {