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 <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>

Author: BridgeAR

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',

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();

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);

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]) {

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) => {

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);

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);

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);

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`.

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;

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;

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;

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);
     }
   }

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();
 }
 

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);

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];

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, {

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;

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)}`
     }
   );
 });

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',
   }
 );

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);

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)}`
       }
     )
   );

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'
     }
   );
 

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)}`
           }
         )
       );

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'"
     }
   );
 

test/parallel/test-http2-server-settimeout-no-callback.js

@@ -6,23 +6,23 @@ if (!common.hasCrypto)
 
 const assert = require('assert');
 const http2 = require('http2');
+const { inspect } = require('util');
 
 // Verify that setTimeout callback verifications work correctly
 const verifyCallbacks = (server) => {
   const testTimeout = 10;
-  const notFunctions = [true, 1, {}, [], null, 'test'];
-  const invalidCallBackError = {
-    type: TypeError,
-    code: 'ERR_INVALID_CALLBACK',
-    message: 'Callback must be a function'
-  };
 
-  notFunctions.forEach((notFunction) =>
+  [true, 1, {}, [], null, 'test'].forEach((notFunction) => {
     common.expectsError(
       () => server.setTimeout(testTimeout, notFunction),
-      invalidCallBackError
-    )
-  );
+      {
+        type: TypeError,
+        code: 'ERR_INVALID_CALLBACK',
+        message: 'Callback must be a function. ' +
+                 `Received ${inspect(notFunction)}`
+      }
+    );
+  });
 
   // No callback
   const returnedVal = server.setTimeout(testTimeout);

test/parallel/test-http2-timeouts.js

@@ -29,15 +29,15 @@ server.on('stream', common.mustCall((stream) => {
     {
       code: 'ERR_INVALID_CALLBACK',
       type: TypeError,
-      message: 'Callback must be a function'
+      message: 'Callback must be a function. Received Symbol(test)'
     }
   );
   common.expectsError(
     () => stream.setTimeout(100, {}),
     {
       code: 'ERR_INVALID_CALLBACK',
       type: TypeError,
-      message: 'Callback must be a function'
+      message: 'Callback must be a function. Received {}'
     }
   );
 }));

test/parallel/test-net-socket-timeout.js

@@ -23,6 +23,7 @@
 const common = require('../common');
 const net = require('net');
 const assert = require('assert');
+const { inspect } = require('util');
 
 // Verify that invalid delays throw
 const s = new net.Socket();
@@ -59,7 +60,8 @@ for (let i = 0; i < invalidCallbacks.length; i++) {
       {
         code: 'ERR_INVALID_CALLBACK',
         type: TypeError,
-        message: 'Callback must be a function'
+        message: 'Callback must be a function. ' +
+                 `Received ${inspect(invalidCallbacks[i])}`
       }
     )
   );

test/parallel/test-performanceobserver.js

@@ -4,6 +4,7 @@
 const common = require('../common');
 const Countdown = require('../common/countdown');
 const assert = require('assert');
+const { inspect } = require('util');
 const { internalBinding } = require('internal/test/binding');
 const {
   observerCounts: counts
@@ -30,12 +31,14 @@ assert.strictEqual(counts[NODE_PERFORMANCE_ENTRY_TYPE_FUNCTION], 0);
 
 {
   [1, null, undefined, {}, [], Infinity].forEach((i) => {
-    common.expectsError(() => new PerformanceObserver(i),
-                        {
-                          code: 'ERR_INVALID_CALLBACK',
-                          type: TypeError,
-                          message: 'Callback must be a function'
-                        });
+    common.expectsError(
+      () => new PerformanceObserver(i),
+      {
+        code: 'ERR_INVALID_CALLBACK',
+        type: TypeError,
+        message: `Callback must be a function. Received ${inspect(i)}`
+      }
+    );
   });
   const observer = new PerformanceObserver(common.mustNotCall());
 

test/parallel/test-process-next-tick.js

@@ -21,6 +21,7 @@
 
 'use strict';
 const common = require('../common');
+const { inspect } = require('util');
 const N = 2;
 
 function cb() {
@@ -38,10 +39,12 @@ process.on('exit', function() {
 });
 
 [null, 1, 'test', {}, [], Infinity, true].forEach((i) => {
-  common.expectsError(() => process.nextTick(i),
-                      {
-                        code: 'ERR_INVALID_CALLBACK',
-                        type: TypeError,
-                        message: 'Callback must be a function'
-                      });
+  common.expectsError(
+    () => process.nextTick(i),
+    {
+      code: 'ERR_INVALID_CALLBACK',
+      type: TypeError,
+      message: `Callback must be a function. Received ${inspect(i)}`
+    }
+  );
 });

test/parallel/test-timers-refresh.js

@@ -6,6 +6,7 @@ const common = require('../common');
 
 const { strictEqual } = require('assert');
 const { setUnrefTimeout } = require('internal/timers');
+const { inspect } = require('util');
 
 // Schedule the unrefed cases first so that the later case keeps the event loop
 // active.
@@ -32,14 +33,14 @@ const { setUnrefTimeout } = require('internal/timers');
 
 // Should throw with non-functions
 {
-  const expectedError = {
-    code: 'ERR_INVALID_CALLBACK',
-    message: 'Callback must be a function'
-  };
-
   [null, true, false, 0, 1, NaN, '', 'foo', {}, Symbol()].forEach((cb) => {
-    common.expectsError(() => setUnrefTimeout(cb),
-                        expectedError);
+    common.expectsError(
+      () => setUnrefTimeout(cb),
+      {
+        code: 'ERR_INVALID_CALLBACK',
+        message: `Callback must be a function. Received ${inspect(cb)}`
+      }
+    );
   });
 }
 

test/sequential/test-inspector-module.js

@@ -5,6 +5,7 @@ const common = require('../common');
 common.skipIfInspectorDisabled();
 
 const { Session } = require('inspector');
+const { inspect } = require('util');
 
 const session = new Session();
 
@@ -52,7 +53,7 @@ session.post('Runtime.evaluate', { expression: '2 + 2' });
     {
       code: 'ERR_INVALID_CALLBACK',
       type: TypeError,
-      message: 'Callback must be a function'
+      message: `Callback must be a function. Received ${inspect(i)}`
     }
   );
 });