fs: throw on invalid callbacks for async functions

If an asynchronous function is passed no callback function, there is no
way to return the result. This patch throws an error if the callback
passed is not valid or none passed at all.

PR-URL: https://github.com/nodejs/node/pull/12562

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

8 files changed, 11 insertions, 28 deletions

diff --git a/test/fixtures/test-fs-readfile-error.js b/test/fixtures/test-fs-readfile-error.js
index 3f8d9a731e..9bac28f8ef 100644
--- a/test/fixtures/test-fs-readfile-error.js
+++ b/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
diff --git a/test/parallel/test-fs-access.js b/test/parallel/test-fs-access.js
index f378824cbc..9ea29c7141 100644
--- a/test/parallel/test-fs-access.js
+++ b/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);
diff --git a/test/parallel/test-fs-link.js b/test/parallel/test-fs-link.js
index 525392aa2b..65df0f82eb 100644
--- a/test/parallel/test-fs-link.js
+++ b/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/
 );
diff --git a/test/parallel/test-fs-make-callback.js b/test/parallel/test-fs-make-callback.js
index 8a19e1cc96..aed5811fc7 100644
--- a/test/parallel/test-fs-make-callback.js
+++ b/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);
diff --git a/test/parallel/test-fs-makeStatsCallback.js b/test/parallel/test-fs-makeStatsCallback.js
index 84a46bb72e..5f923d4786 100644
--- a/test/parallel/test-fs-makeStatsCallback.js
+++ b/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);
diff --git a/test/parallel/test-fs-mkdtemp.js b/test/parallel/test-fs-mkdtemp.js
index d4773aa37b..82c58dbf78 100644
--- a/test/parallel/test-fs-mkdtemp.js
+++ b/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-'), {}));
diff --git a/test/parallel/test-fs-readfile-error.js b/test/parallel/test-fs-readfile-error.js
index 208b478c12..074d7d4af2 100644
--- a/test/parallel/test-fs-readfile-error.js
+++ b/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) => {
diff --git a/test/parallel/test-fs-write-no-fd.js b/test/parallel/test-fs-write-no-fd.js
index 0aaec4b733..576457203e 100644
--- a/test/parallel/test-fs-write-no-fd.js
+++ b/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/);