fs: throw on invalid callbacks for async functions

doc/api/deprecations.md

@@ -150,9 +150,10 @@ explicitly via error event handlers set on the domain instead.
 <a id="DEP0013"></a>
 ### DEP0013: fs async function without callback
 
-Type: Runtime
+Type: End-of-Life
 
-Calling an asynchronous function without a callback is deprecated.
+Calling an asynchronous function without a callback will throw a `TypeError`
+v8.0.0 onwards. Refer: [PR 12562](https://github.com/nodejs/node/pull/12562)
 
 <a id="DEP0014"></a>
 ### DEP0014: fs.read legacy String interface

lib/fs.js

@@ -58,7 +58,6 @@ const kMaxLength = require('buffer').kMaxLength;
 
 const isWindows = process.platform === 'win32';
 
-const DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG);
 const errnoException = util._errnoException;
 
 function getOptions(options, defaultOptions) {
@@ -88,48 +87,26 @@ function copyObject(source) {
   return target;
 }
 
-function rethrow() {
-  // TODO(thefourtheye) Throw error instead of warning in major version > 7
-  process.emitWarning(
-    'Calling an asynchronous function without callback is deprecated.',
-    'DeprecationWarning', 'DEP0013', rethrow
-  );
-
-  // Only enable in debug mode. A backtrace uses ~1000 bytes of heap space and
-  // is fairly slow to generate.
-  if (DEBUG) {
-    var backtrace = new Error();
-    return function(err) {
-      if (err) {
-        backtrace.stack = err.name + ': ' + err.message +
-                          backtrace.stack.substr(backtrace.name.length);
-        throw backtrace;
-      }
-    };
-  }
-
-  return function(err) {
-    if (err) {
-      throw err;  // Forgot a callback but don't know where? Use NODE_DEBUG=fs
-    }
-  };
+var internalErrors;
+function lazyErrors() {
+  if (!internalErrors)
+    internalErrors = require('internal/errors');
+  return internalErrors;
 }
 
 function maybeCallback(cb) {
-  return typeof cb === 'function' ? cb : rethrow();
+  if (typeof cb === 'function')
+    return cb;
+  else
+    throw new (lazyErrors().TypeError)('ERR_INVALID_CALLBACK');
 }
 
 // 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 (cb === undefined) {
-    return rethrow();
-  }
-
-  if (typeof cb !== 'function') {
-    throw new TypeError('"callback" argument must be a function');
-  }
+  if (typeof cb !== 'function')
+    throw new (lazyErrors().TypeError)('ERR_INVALID_CALLBACK');
 
   return function() {
     return cb.apply(null, arguments);
@@ -140,13 +117,8 @@ function makeCallback(cb) {
 // an optimization, since the data passed back to the callback needs to be
 // transformed anyway.
 function makeStatsCallback(cb) {
-  if (cb === undefined) {
-    return rethrow();
-  }
-
-  if (typeof cb !== 'function') {
-    throw new TypeError('"callback" argument must be a function');
-  }
+  if (typeof cb !== 'function')
+    throw new (lazyErrors().TypeError)('ERR_INVALID_CALLBACK');
 
   return function(err) {
     if (err) return cb(err);
@@ -268,10 +240,10 @@ fs.access = function(path, mode, callback) {
   if (typeof mode === 'function') {
     callback = mode;
     mode = fs.F_OK;
-  } else if (typeof callback !== 'function') {
-    throw new TypeError('"callback" argument must be a function');
   }
 
+  callback = makeCallback(callback);
+
   if (handleError((path = getPathFromURL(path)), callback))
     return;
 
@@ -280,7 +252,7 @@ fs.access = function(path, mode, callback) {
 
   mode = mode | 0;
   var req = new FSReqWrap();
-  req.oncomplete = makeCallback(callback);
+  req.oncomplete = callback;
   binding.access(pathModule._makeLong(path), mode, req);
 };
 

test/fixtures/test-fs-readfile-error.js

@@ -19,4 +19,4 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-require('fs').readFile('/');  // throws EISDIR
+require('fs').readFileSync('/');  // throws EISDIR

test/parallel/test-fs-access.js

@@ -87,11 +87,11 @@ assert.throws(() => {
 
 assert.throws(() => {
   fs.access(__filename, fs.F_OK);
-}, /^TypeError: "callback" argument must be a function$/);
+}, common.expectsError({code: 'ERR_INVALID_CALLBACK'}));
 
 assert.throws(() => {
   fs.access(__filename, fs.F_OK, {});
-}, /^TypeError: "callback" argument must be a function$/);
+}, common.expectsError({code: 'ERR_INVALID_CALLBACK'}));
 
 assert.doesNotThrow(() => {
   fs.accessSync(__filename);

test/parallel/test-fs-link.js

@@ -23,14 +23,14 @@ fs.link(srcPath, dstPath, common.mustCall(callback));
 
 assert.throws(
   function() {
-    fs.link();
+    fs.link(undefined, undefined, common.mustNotCall());
   },
   /src must be a string or Buffer/
 );
 
 assert.throws(
   function() {
-    fs.link('abc');
+    fs.link('abc', undefined, common.mustNotCall());
   },
   /dest must be a string or Buffer/
 );

test/parallel/test-fs-make-callback.js

@@ -2,11 +2,10 @@
 const common = require('../common');
 const assert = require('assert');
 const fs = require('fs');
-const cbTypeError = /^TypeError: "callback" argument must be a function$/;
+const cbTypeError = common.expectsError({code: 'ERR_INVALID_CALLBACK'});
 const callbackThrowValues = [null, true, false, 0, 1, 'foo', /foo/, [], {}];
 
 const { sep } = require('path');
-const warn = 'Calling an asynchronous function without callback is deprecated.';
 
 common.refreshTmpDir();
 
@@ -17,11 +16,6 @@ function testMakeCallback(cb) {
   };
 }
 
-common.expectWarning('DeprecationWarning', warn);
-
-// Passing undefined/nothing calls rethrow() internally, which emits a warning
-assert.doesNotThrow(testMakeCallback());
-
 function invalidCallbackThrowsTests() {
   callbackThrowValues.forEach((value) => {
     assert.throws(testMakeCallback(value), cbTypeError);

test/parallel/test-fs-makeStatsCallback.js

@@ -2,9 +2,8 @@
 const common = require('../common');
 const assert = require('assert');
 const fs = require('fs');
-const cbTypeError = /^TypeError: "callback" argument must be a function$/;
+const cbTypeError = common.expectsError({code: 'ERR_INVALID_CALLBACK'});
 const callbackThrowValues = [null, true, false, 0, 1, 'foo', /foo/, [], {}];
-const warn = 'Calling an asynchronous function without callback is deprecated.';
 
 function testMakeStatsCallback(cb) {
   return function() {
@@ -13,14 +12,9 @@ function testMakeStatsCallback(cb) {
   };
 }
 
-common.expectWarning('DeprecationWarning', warn);
-
 // Verify the case where a callback function is provided
 assert.doesNotThrow(testMakeStatsCallback(common.noop));
 
-// Passing undefined/nothing calls rethrow() internally, which emits a warning
-assert.doesNotThrow(testMakeStatsCallback());
-
 function invalidCallbackThrowsTests() {
   callbackThrowValues.forEach((value) => {
     assert.throws(testMakeStatsCallback(value), cbTypeError);

test/parallel/test-fs-mkdtemp.js

@@ -29,8 +29,3 @@ fs.mkdtemp(path.join(common.tmpDir, 'bar.'), common.mustCall(handler));
 // Same test as above, but making sure that passing an options object doesn't
 // affect the way the callback function is handled.
 fs.mkdtemp(path.join(common.tmpDir, 'bar.'), {}, common.mustCall(handler));
-
-// Making sure that not passing a callback doesn't crash, as a default function
-// is passed internally.
-assert.doesNotThrow(() => fs.mkdtemp(path.join(common.tmpDir, 'bar-')));
-assert.doesNotThrow(() => fs.mkdtemp(path.join(common.tmpDir, 'bar-'), {}));

test/parallel/test-fs-readfile-error.js

@@ -46,7 +46,7 @@ function test(env, cb) {
 
 test({ NODE_DEBUG: '' }, common.mustCall((data) => {
   assert(/EISDIR/.test(data));
-  assert(!/test-fs-readfile-error/.test(data));
+  assert(/test-fs-readfile-error/.test(data));
 }));
 
 test({ NODE_DEBUG: 'fs' }, common.mustCall((data) => {

test/parallel/test-fs-write-no-fd.js

@@ -1,12 +1,12 @@
 'use strict';
-require('../common');
+const common = require('../common');
 const fs = require('fs');
 const assert = require('assert');
 
 assert.throws(function() {
-  fs.write(null, Buffer.allocUnsafe(1), 0, 1);
+  fs.write(null, Buffer.allocUnsafe(1), 0, 1, common.mustNotCall());
 }, /TypeError/);
 
 assert.throws(function() {
-  fs.write(null, '1', 0, 1);
+  fs.write(null, '1', 0, 1, common.mustNotCall());
 }, /TypeError/);