From 9ebba76c03f0465f4c11ae79da59909cefb5f828 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <codebytere@github.com>
Date: Thu, 7 Feb 2019 10:25:20 -0800
Subject: [PATCH] chore: improve existing error preservation in promisify
 (#16815)

This PR better preserves existing behavior in `deprecate.promisify()` in the cases where the promise fails. Previously, if a callback was only called with `data` instead of `err, data` and the promise was rejected, `data` would be populated with `err`, which could be confusing to users. This makes it such that `err` is called back on promise rejection if a callback is called with `err, data` a la Node.js.
---
 lib/common/api/deprecate.js   |  4 +++-
 spec/api-deprecations-spec.js | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/lib/common/api/deprecate.js b/lib/common/api/deprecate.js
index 42218003f78c..e61fb2b9fef4 100644
--- a/lib/common/api/deprecate.js
+++ b/lib/common/api/deprecate.js
@@ -89,7 +89,9 @@ const deprecate = {
             cb.length === 2 ? cb(null, res) : cb(res)
           })
         }, err => {
-          process.nextTick(() => cb(err))
+          process.nextTick(() => {
+            cb.length === 2 ? cb(err) : cb()
+          })
         })
     }
   },
diff --git a/spec/api-deprecations-spec.js b/spec/api-deprecations-spec.js
index 7236107f262d..ebaa616fb872 100644
--- a/spec/api-deprecations-spec.js
+++ b/spec/api-deprecations-spec.js
@@ -145,6 +145,24 @@ describe('deprecations', () => {
       expect(warnings).to.have.lengthOf(0)
     })
 
+    it('only calls back an error if the callback is called with (err, data)', (done) => {
+      enableCallbackWarnings()
+      let erringPromiseFunc = () => new Promise((resolve, reject) => {
+        reject(new Error('fail'))
+      })
+      erringPromiseFunc = deprecate.promisify(erringPromiseFunc)
+
+      erringPromiseFunc((err, data) => {
+        expect(data).to.be.an('undefined')
+        expect(err).to.be.an.instanceOf(Error).with.property('message', 'fail')
+        erringPromiseFunc(data => {
+          expect(data).to.not.be.an.instanceOf(Error)
+          expect(data).to.be.an('undefined')
+          done()
+        })
+      })
+    })
+
     it('warns exactly once for callback-based invocations', (done) => {
       enableCallbackWarnings()
       promiseFunc = deprecate.promisify(promiseFunc)