errors: add more information in case of invalid callbacks

This adds the actual callback that is passed through to the error message in case an ERR_INVALID_CALLBACK error is thrown. PR-URL: #27048 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
nodejs · Apr 4, 2019 · 3b04496 · 3b04496
1 parent a9bf665
commit 3b04496
Show file tree

Hide file tree

Showing 32 changed files with 90 additions and 73 deletions.
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -633,7 +633,7 @@ TLSSocket.prototype.renegotiate = function(options, callback) {
   if (options === null || typeof options !== 'object')
     throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
   if (callback !== undefined && typeof callback !== 'function')
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
 
   debug('%s renegotiate()',
         this._tlsOptions.isServer ? 'server' : 'client',

diff --git a/lib/dns.js b/lib/dns.js
@@ -98,7 +98,7 @@ function lookup(hostname, options, callback) {
     callback = options;
     family = 0;
   } else if (typeof callback !== 'function') {
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
   } else if (options !== null && typeof options === 'object') {
     hints = options.hints >>> 0;
     family = options.family >>> 0;
@@ -174,7 +174,7 @@ function lookupService(hostname, port, callback) {
     throw new ERR_SOCKET_BAD_PORT(port);
 
   if (typeof callback !== 'function')
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
 
   port = +port;
 
@@ -213,7 +213,7 @@ function resolver(bindingName) {
 
     validateString(name, 'name');
     if (typeof callback !== 'function') {
-      throw new ERR_INVALID_CALLBACK();
+      throw new ERR_INVALID_CALLBACK(callback);
     }
 
     const req = new QueryReqWrap();

diff --git a/lib/fs.js b/lib/fs.js
@@ -137,15 +137,15 @@ function maybeCallback(cb) {
   if (typeof cb === 'function')
     return cb;
 
-  throw new ERR_INVALID_CALLBACK();
+  throw new ERR_INVALID_CALLBACK(cb);
 }
 
 // Ensure that callbacks run in the global context. Only use this function
 // for callbacks that are passed to the binding layer, callbacks that are
 // invoked from JS already run in the proper scope.
 function makeCallback(cb) {
   if (typeof cb !== 'function') {
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(cb);
   }
 
   return (...args) => {
@@ -158,7 +158,7 @@ function makeCallback(cb) {
 // transformed anyway.
 function makeStatsCallback(cb) {
   if (typeof cb !== 'function') {
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(cb);
   }
 
   return (err, stats) => {
@@ -1749,7 +1749,7 @@ function copyFile(src, dest, flags, callback) {
     callback = flags;
     flags = 0;
   } else if (typeof callback !== 'function') {
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
   }
 
   src = toPathIfFileURL(src);

diff --git a/lib/inspector.js b/lib/inspector.js
@@ -78,7 +78,7 @@ class Session extends EventEmitter {
       throw new ERR_INVALID_ARG_TYPE('params', 'Object', params);
     }
     if (callback && typeof callback !== 'function') {
-      throw new ERR_INVALID_CALLBACK();
+      throw new ERR_INVALID_CALLBACK(callback);
     }
 
     if (!this[connectionSymbol]) {

diff --git a/lib/internal/crypto/keygen.js b/lib/internal/crypto/keygen.js
@@ -46,7 +46,7 @@ function generateKeyPair(type, options, callback) {
   const impl = check(type, options);
 
   if (typeof callback !== 'function')
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
 
   const wrap = new AsyncWrap(Providers.KEYPAIRGENREQUEST);
   wrap.ondone = (ex, pubkey, privkey) => {

diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js
@@ -26,7 +26,7 @@ function pbkdf2(password, salt, iterations, keylen, digest, callback) {
     check(password, salt, iterations, keylen, digest));
 
   if (typeof callback !== 'function')
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
 
   const encoding = getDefaultEncoding();
   const keybuf = Buffer.alloc(keylen);

diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js
@@ -47,7 +47,7 @@ function assertSize(size, elementSize, offset, length) {
 function randomBytes(size, cb) {
   size = assertSize(size, 1, 0, Infinity);
   if (cb !== undefined && typeof cb !== 'function')
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(cb);
 
   const buf = Buffer.alloc(size);
 
@@ -95,7 +95,7 @@ function randomFill(buf, offset, size, cb) {
     cb = size;
     size = buf.byteLength - offset;
   } else if (typeof cb !== 'function') {
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(cb);
   }
 
   offset = assertOffset(offset, elementSize, buf.byteLength);

diff --git a/lib/internal/crypto/scrypt.js b/lib/internal/crypto/scrypt.js
@@ -32,7 +32,7 @@ function scrypt(password, salt, keylen, options, callback = defaults) {
   ({ password, salt, keylen } = options);
 
   if (typeof callback !== 'function')
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
 
   const encoding = getDefaultEncoding();
   const keybuf = Buffer.alloc(keylen);

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -854,7 +854,8 @@ E('ERR_INVALID_ARG_VALUE', (name, value, reason = 'is invalid') => {
 E('ERR_INVALID_ASYNC_ID', 'Invalid %s value: %s', RangeError);
 E('ERR_INVALID_BUFFER_SIZE',
   'Buffer size must be a multiple of %s', RangeError);
-E('ERR_INVALID_CALLBACK', 'Callback must be a function', TypeError);
+E('ERR_INVALID_CALLBACK',
+  'Callback must be a function. Received %O', TypeError);
 E('ERR_INVALID_CHAR',
   // Using a default argument here is important so the argument is not counted
   // towards `Function#length`.

diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js
@@ -701,7 +701,7 @@ class Http2ServerResponse extends Stream {
 
   createPushResponse(headers, callback) {
     if (typeof callback !== 'function')
-      throw new ERR_INVALID_CALLBACK();
+      throw new ERR_INVALID_CALLBACK(callback);
     if (this[kState].closed) {
       process.nextTick(callback, new ERR_HTTP2_INVALID_STREAM());
       return;

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -1058,7 +1058,7 @@ class Http2Session extends EventEmitter {
       throw new ERR_HTTP2_PING_LENGTH();
     }
     if (typeof callback !== 'function')
-      throw new ERR_INVALID_CALLBACK();
+      throw new ERR_INVALID_CALLBACK(callback);
 
     const cb = pingCallback(callback);
     if (this.connecting || this.closed) {
@@ -1148,7 +1148,7 @@ class Http2Session extends EventEmitter {
     validateSettings(settings);
 
     if (callback && typeof callback !== 'function')
-      throw new ERR_INVALID_CALLBACK();
+      throw new ERR_INVALID_CALLBACK(callback);
     debug(`Http2Session ${sessionName(this[kType])}: sending settings`);
 
     this[kState].pendingAck++;
@@ -1900,7 +1900,7 @@ class Http2Stream extends Duplex {
     if (code < 0 || code > kMaxInt)
       throw new ERR_OUT_OF_RANGE('code', `>= 0 && <= ${kMaxInt}`, code);
     if (callback !== undefined && typeof callback !== 'function')
-      throw new ERR_INVALID_CALLBACK();
+      throw new ERR_INVALID_CALLBACK(callback);
 
     if (this.closed)
       return;
@@ -2256,7 +2256,7 @@ class ServerHttp2Stream extends Http2Stream {
     }
 
     if (typeof callback !== 'function')
-      throw new ERR_INVALID_CALLBACK();
+      throw new ERR_INVALID_CALLBACK(callback);
 
     assertIsObject(options, 'options');
     options = { ...options };
@@ -2690,7 +2690,7 @@ class Http2SecureServer extends TLSServer {
     this.timeout = msecs;
     if (callback !== undefined) {
       if (typeof callback !== 'function')
-        throw new ERR_INVALID_CALLBACK();
+        throw new ERR_INVALID_CALLBACK(callback);
       this.on('timeout', callback);
     }
     return this;
@@ -2711,7 +2711,7 @@ class Http2Server extends NETServer {
     this.timeout = msecs;
     if (callback !== undefined) {
       if (typeof callback !== 'function')
-        throw new ERR_INVALID_CALLBACK();
+        throw new ERR_INVALID_CALLBACK(callback);
       this.on('timeout', callback);
     }
     return this;

diff --git a/lib/internal/process/task_queues.js b/lib/internal/process/task_queues.js
@@ -115,7 +115,7 @@ class TickObject {
 // exit since the callback would not have a chance to be executed.
 function nextTick(callback) {
   if (typeof callback !== 'function')
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
 
   if (process._exiting)
     return;

diff --git a/lib/internal/stream_base_commons.js b/lib/internal/stream_base_commons.js
@@ -214,7 +214,7 @@ function setStreamTimeout(msecs, callback) {
   if (msecs === 0) {
     if (callback !== undefined) {
       if (typeof callback !== 'function')
-        throw new ERR_INVALID_CALLBACK();
+        throw new ERR_INVALID_CALLBACK(callback);
       this.removeListener('timeout', callback);
     }
   } else {
@@ -223,7 +223,7 @@ function setStreamTimeout(msecs, callback) {
 
     if (callback !== undefined) {
       if (typeof callback !== 'function')
-        throw new ERR_INVALID_CALLBACK();
+        throw new ERR_INVALID_CALLBACK(callback);
       this.once('timeout', callback);
     }
   }

diff --git a/lib/internal/streams/pipeline.js b/lib/internal/streams/pipeline.js
@@ -58,7 +58,7 @@ function popCallback(streams) {
   // a single stream. Therefore optimize for the average case instead of
   // checking for length === 0 as well.
   if (typeof streams[streams.length - 1] !== 'function')
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(streams[streams.length - 1]);
   return streams.pop();
 }
 

diff --git a/lib/internal/timers.js b/lib/internal/timers.js
@@ -351,7 +351,7 @@ function insert(item, refed, start) {
 function setUnrefTimeout(callback, after) {
   // Type checking identical to setTimeout()
   if (typeof callback !== 'function') {
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
   }
 
   const timer = new Timeout(callback, after, undefined, false);

diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -1092,7 +1092,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', {
       throw new ERR_INVALID_THIS('URLSearchParams');
     }
     if (typeof callback !== 'function') {
-      throw new ERR_INVALID_CALLBACK();
+      throw new ERR_INVALID_CALLBACK(callback);
     }
 
     let list = this[searchParams];

diff --git a/lib/perf_hooks.js b/lib/perf_hooks.js
@@ -283,7 +283,7 @@ let gcTrackingIsEnabled = false;
 class PerformanceObserver extends AsyncResource {
   constructor(callback) {
     if (typeof callback !== 'function') {
-      throw new ERR_INVALID_CALLBACK();
+      throw new ERR_INVALID_CALLBACK(callback);
     }
     super('PerformanceObserver');
     Object.defineProperties(this, {

diff --git a/lib/timers.js b/lib/timers.js
@@ -122,7 +122,7 @@ function enroll(item, msecs) {
 
 function setTimeout(callback, after, arg1, arg2, arg3) {
   if (typeof callback !== 'function') {
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
   }
 
   var i, args;
@@ -168,7 +168,7 @@ function clearTimeout(timer) {
 
 function setInterval(callback, repeat, arg1, arg2, arg3) {
   if (typeof callback !== 'function') {
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
   }
 
   var i, args;
@@ -255,7 +255,7 @@ const Immediate = class Immediate {
 
 function setImmediate(callback, arg1, arg2, arg3) {
   if (typeof callback !== 'function') {
-    throw new ERR_INVALID_CALLBACK();
+    throw new ERR_INVALID_CALLBACK(callback);
   }
 
   var i, args;

diff --git a/test/parallel/test-crypto-random.js b/test/parallel/test-crypto-random.js
@@ -29,6 +29,7 @@ if (!common.hasCrypto)
 const assert = require('assert');
 const crypto = require('crypto');
 const { kMaxLength } = require('buffer');
+const { inspect } = require('util');
 
 const kMaxUint32 = Math.pow(2, 32) - 1;
 const kMaxPossibleLength = Math.min(kMaxLength, kMaxUint32);
@@ -292,7 +293,7 @@ assert.throws(
     {
       code: 'ERR_INVALID_CALLBACK',
       type: TypeError,
-      message: 'Callback must be a function',
+      message: `Callback must be a function. Received ${inspect(i)}`
     });
 });
 
@@ -302,7 +303,7 @@ assert.throws(
     {
       code: 'ERR_INVALID_CALLBACK',
       type: TypeError,
-      message: 'Callback must be a function',
+      message: `Callback must be a function. Received ${inspect(i)}`
     }
   );
 });

diff --git a/test/parallel/test-fs-read.js b/test/parallel/test-fs-read.js
@@ -71,7 +71,7 @@ test(new Uint8Array(expected.length),
 assert.throws(
   () => fs.read(fd, Buffer.alloc(1), 0, 1, 0),
   {
-    message: 'Callback must be a function',
+    message: 'Callback must be a function. Received undefined',
     code: 'ERR_INVALID_CALLBACK',
   }
 );

diff --git a/test/parallel/test-http2-client-rststream-before-connect.js b/test/parallel/test-http2-client-rststream-before-connect.js
@@ -5,6 +5,7 @@ if (!common.hasCrypto)
   common.skip('missing crypto');
 const assert = require('assert');
 const h2 = require('http2');
+const { inspect } = require('util');
 
 const server = h2.createServer();
 server.on('stream', (stream) => {
@@ -35,7 +36,7 @@ server.listen(0, common.mustCall(() => {
       {
         type: TypeError,
         code: 'ERR_INVALID_CALLBACK',
-        message: 'Callback must be a function'
+        message: `Callback must be a function. Received ${inspect(notFunction)}`
       }
     );
     assert.strictEqual(req.closed, false);

diff --git a/test/parallel/test-http2-client-settings-before-connect.js b/test/parallel/test-http2-client-settings-before-connect.js
@@ -4,6 +4,7 @@ const common = require('../common');
 if (!common.hasCrypto)
   common.skip('missing crypto');
 const h2 = require('http2');
+const { inspect } = require('util');
 
 const server = h2.createServer();
 
@@ -51,7 +52,8 @@ server.listen(0, common.mustCall(() => {
       {
         type: TypeError,
         code: 'ERR_INVALID_CALLBACK',
-        message: 'Callback must be a function'
+        message:
+          `Callback must be a function. Received ${inspect(invalidCallback)}`
       }
     )
   );

diff --git a/test/parallel/test-http2-compat-serverresponse-createpushresponse.js b/test/parallel/test-http2-compat-serverresponse-createpushresponse.js
@@ -24,7 +24,7 @@ const server = h2.createServer((request, response) => {
     {
       code: 'ERR_INVALID_CALLBACK',
       type: TypeError,
-      message: 'Callback must be a function'
+      message: 'Callback must be a function. Received undefined'
     }
   );
 

diff --git a/test/parallel/test-http2-ping.js b/test/parallel/test-http2-ping.js
@@ -7,6 +7,7 @@ if (!common.hasCrypto)
 const async_hooks = require('async_hooks');
 const assert = require('assert');
 const http2 = require('http2');
+const { inspect } = require('util');
 
 const pings = new Set();
 const events = [0, 0, 0, 0];
@@ -119,7 +120,8 @@ server.listen(0, common.mustCall(() => {
           {
             type: TypeError,
             code: 'ERR_INVALID_CALLBACK',
-            message: 'Callback must be a function'
+            message: 'Callback must be a function. ' +
+                     `Received ${inspect(invalidCallback)}`
           }
         )
       );

diff --git a/test/parallel/test-http2-server-push-stream-errors-args.js b/test/parallel/test-http2-server-push-stream-errors-args.js
@@ -22,7 +22,7 @@ server.on('stream', common.mustCall((stream, headers) => {
     }, {}, 'callback'),
     {
       code: 'ERR_INVALID_CALLBACK',
-      message: 'Callback must be a function'
+      message: "Callback must be a function. Received 'callback'"
     }
   );