errors: convert 'fs'

covert lib/fs.js over to using lib/internal/errors.js i have not addressed the cases that use errnoException(), for reasons described in GH-12926 - throw the ERR_INVALID_CALLBACK error when the the callback is invalid - replace the ['object', 'string'] with ['string', 'object'] in the error constructor call, to better match the previous err msg in the getOptions() function - add error ERR_VALUE_OUT_OF_RANGE in lib/internal/errors.js, this error is thrown when a numeric value is out of range - document the ERR_VALUE_OUT_OF_RANGE err in errors.md - correct the expected args, in the error thrown in the function fs._toUnixTimestamp() to ['Date', 'time in seconds'] (lib/fs.js) - update the listener error type in the fs.watchFile() function, from Error to TypeError (lib/fs.js) - update errors from ERR_INVALID_OPT_VALUE to ERR_INVALID_ARG_TYPE in the functions fs.ReadStream() and fs.WriteStream(), for the cases of range errors use the new error: ERR_VALUE_OUT_OF_RANGE (lib/fs.js) PR-URL: #15043 Refs: #11273 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
nodejs · Aug 31, 2017 · 219932a · 219932a
1 parent a517466
commit 219932a
Show file tree

Hide file tree

Showing 17 changed files with 246 additions and 116 deletions.
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -1136,6 +1136,11 @@ Used when an attempt is made to launch a Node.js process with an unknown
 by errors in user code, although it is not impossible. Occurrences of this error
 are most likely an indication of a bug within Node.js itself.
 
+<a id="ERR_VALUE_OUT_OF_RANGE"></a>
+### ERR_VALUE_OUT_OF_RANGE
+
+Used when a number value is out of range.
+
 <a id="ERR_V8BREAKITERATOR"></a>
 ### ERR_V8BREAKITERATOR
 

diff --git a/lib/fs.js b/lib/fs.js
@@ -33,6 +33,7 @@ const { isUint8Array, createPromise, promiseResolve } = process.binding('util');
 const binding = process.binding('fs');
 const fs = exports;
 const Buffer = require('buffer').Buffer;
+const errors = require('internal/errors');
 const Stream = require('stream').Stream;
 const EventEmitter = require('events');
 const FSReqWrap = binding.FSReqWrap;
@@ -72,8 +73,10 @@ function getOptions(options, defaultOptions) {
     defaultOptions.encoding = options;
     options = defaultOptions;
   } else if (typeof options !== 'object') {
-    throw new TypeError('"options" must be a string or an object, got ' +
-                        typeof options + ' instead.');
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
+                               'options',
+                               ['string', 'object'],
+                               options);
   }
 
   if (options.encoding !== 'buffer')
@@ -128,7 +131,7 @@ function makeCallback(cb) {
   }
 
   if (typeof cb !== 'function') {
-    throw new TypeError('"callback" argument must be a function');
+    throw new errors.TypeError('ERR_INVALID_CALLBACK');
   }
 
   return function() {
@@ -145,7 +148,7 @@ function makeStatsCallback(cb) {
   }
 
   if (typeof cb !== 'function') {
-    throw new TypeError('"callback" argument must be a function');
+    throw new errors.TypeError('ERR_INVALID_CALLBACK');
   }
 
   return function(err) {
@@ -156,8 +159,11 @@ function makeStatsCallback(cb) {
 
 function nullCheck(path, callback) {
   if (('' + path).indexOf('\u0000') !== -1) {
-    var er = new Error('Path must be a string without null bytes');
-    er.code = 'ENOENT';
+    const er = new errors.Error('ERR_INVALID_ARG_TYPE',
+                                'path',
+                                'string without null bytes',
+                                path);
+
     if (typeof callback !== 'function')
       throw er;
     process.nextTick(callback, er);
@@ -274,7 +280,7 @@ fs.access = function(path, mode, callback) {
     callback = mode;
     mode = fs.F_OK;
   } else if (typeof callback !== 'function') {
-    throw new TypeError('"callback" argument must be a function');
+    throw new errors.TypeError('ERR_INVALID_CALLBACK');
   }
 
   if (handleError((path = getPathFromURL(path)), callback))
@@ -1193,7 +1199,10 @@ function toUnixTimestamp(time) {
     // convert to 123.456 UNIX timestamp
     return time.getTime() / 1000;
   }
-  throw new Error('Cannot parse time: ' + time);
+  throw new errors.Error('ERR_INVALID_ARG_TYPE',
+                         'time',
+                         ['Date', 'time in seconds'],
+                         time);
 }
 
 // exported for unit tests, not for public consumption
@@ -1495,7 +1504,10 @@ fs.watchFile = function(filename, options, listener) {
   }
 
   if (typeof listener !== 'function') {
-    throw new Error('"watchFile()" requires a listener function');
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
+                               'listener',
+                               'function',
+                               listener);
   }
 
   stat = statWatchers.get(filename);
@@ -1842,8 +1854,12 @@ fs.realpath = function realpath(p, options, callback) {
 fs.mkdtemp = function(prefix, options, callback) {
   callback = makeCallback(typeof options === 'function' ? options : callback);
   options = getOptions(options, {});
-  if (!prefix || typeof prefix !== 'string')
-    throw new TypeError('filename prefix is required');
+  if (!prefix || typeof prefix !== 'string') {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
+                               'prefix',
+                               'string',
+                               prefix);
+  }
   if (!nullCheck(prefix, callback)) {
     return;
   }
@@ -1856,8 +1872,12 @@ fs.mkdtemp = function(prefix, options, callback) {
 
 
 fs.mkdtempSync = function(prefix, options) {
-  if (!prefix || typeof prefix !== 'string')
-    throw new TypeError('filename prefix is required');
+  if (!prefix || typeof prefix !== 'string') {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
+                               'prefix',
+                               'string',
+                               prefix);
+  }
   options = getOptions(options, {});
   nullCheck(prefix);
   return binding.mkdtemp(prefix + 'XXXXXX', options.encoding);
@@ -1903,16 +1923,26 @@ function ReadStream(path, options) {
 
   if (this.start !== undefined) {
     if (typeof this.start !== 'number') {
-      throw new TypeError('"start" option must be a Number');
+      throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
+                                 'start',
+                                 'number',
+                                 this.start);
     }
     if (this.end === undefined) {
       this.end = Infinity;
     } else if (typeof this.end !== 'number') {
-      throw new TypeError('"end" option must be a Number');
+      throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
+                                 'end',
+                                 'number',
+                                 this.end);
     }
 
     if (this.start > this.end) {
-      throw new Error('"start" option must be <= "end" option');
+      const errVal = `{start: ${this.start}, end: ${this.end}}`;
+      throw new errors.RangeError('ERR_VALUE_OUT_OF_RANGE',
+                                  'start',
+                                  '<= "end"',
+                                  errVal);
     }
 
     this.pos = this.start;
@@ -2069,10 +2099,17 @@ function WriteStream(path, options) {
 
   if (this.start !== undefined) {
     if (typeof this.start !== 'number') {
-      throw new TypeError('"start" option must be a Number');
+      throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
+                                 'start',
+                                 'number',
+                                 this.start);
     }
     if (this.start < 0) {
-      throw new Error('"start" must be >= zero');
+      const errVal = `{start: ${this.start}}`;
+      throw new errors.RangeError('ERR_VALUE_OUT_OF_RANGE',
+                                  'start',
+                                  '>= 0',
+                                  errVal);
     }
 
     this.pos = this.start;

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -264,6 +264,9 @@ E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s');
 E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s');
 E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type');
 E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type');
+E('ERR_VALUE_OUT_OF_RANGE', (start, end, value) => {
+  return `The value of "${start}" must be ${end}. Received "${value}"`;
+});
 E('ERR_V8BREAKITERATOR', 'Full ICU data not installed. ' +
   'See https://github.com/nodejs/node/wiki/Intl');
 E('ERR_VALID_PERFORMANCE_ENTRY_TYPE',

diff --git a/test/parallel/test-file-write-stream3.js b/test/parallel/test-file-write-stream3.js
@@ -176,12 +176,15 @@ function run_test_3() {
 
 const run_test_4 = common.mustCall(function() {
   //  Error: start must be >= zero
-  assert.throws(
-    function() {
-      fs.createWriteStream(filepath, { start: -5, flags: 'r+' });
-    },
-    /"start" must be/
-  );
+  const block = () => {
+    fs.createWriteStream(filepath, { start: -5, flags: 'r+' });
+  };
+  const err = {
+    code: 'ERR_VALUE_OUT_OF_RANGE',
+    message: 'The value of "start" must be >= 0. Received "{start: -5}"',
+    type: RangeError
+  };
+  common.expectsError(block, err);
 });
 
 run_test_1();
diff --git a/test/parallel/test-fs-access.js b/test/parallel/test-fs-access.js
@@ -85,13 +85,23 @@ assert.throws(() => {
   fs.access(100, fs.F_OK, common.mustNotCall());
 }, /^TypeError: path must be a string or Buffer$/);
 
-assert.throws(() => {
-  fs.access(__filename, fs.F_OK);
-}, /^TypeError: "callback" argument must be a function$/);
-
-assert.throws(() => {
-  fs.access(__filename, fs.F_OK, {});
-}, /^TypeError: "callback" argument must be a function$/);
+common.expectsError(
+  () => {
+    fs.access(__filename, fs.F_OK);
+  },
+  {
+    code: 'ERR_INVALID_CALLBACK',
+    type: TypeError
+  });
+
+common.expectsError(
+  () => {
+    fs.access(__filename, fs.F_OK, {});
+  },
+  {
+    code: 'ERR_INVALID_CALLBACK',
+    type: TypeError
+  });
 
 assert.doesNotThrow(() => {
   fs.accessSync(__filename);

diff --git a/test/parallel/test-fs-make-callback.js b/test/parallel/test-fs-make-callback.js
@@ -2,7 +2,6 @@
 const common = require('../common');
 const assert = require('assert');
 const fs = require('fs');
-const cbTypeError = /^TypeError: "callback" argument must be a function$/;
 const callbackThrowValues = [null, true, false, 0, 1, 'foo', /foo/, [], {}];
 
 const { sep } = require('path');
@@ -24,7 +23,10 @@ assert.doesNotThrow(testMakeCallback());
 
 function invalidCallbackThrowsTests() {
   callbackThrowValues.forEach((value) => {
-    assert.throws(testMakeCallback(value), cbTypeError);
+    common.expectsError(testMakeCallback(value), {
+      code: 'ERR_INVALID_CALLBACK',
+      type: TypeError
+    });
   });
 }
 

diff --git a/test/parallel/test-fs-makeStatsCallback.js b/test/parallel/test-fs-makeStatsCallback.js
@@ -2,7 +2,6 @@
 const common = require('../common');
 const assert = require('assert');
 const fs = require('fs');
-const cbTypeError = /^TypeError: "callback" argument must be a function$/;
 const callbackThrowValues = [null, true, false, 0, 1, 'foo', /foo/, [], {}];
 const warn = 'Calling an asynchronous function without callback is deprecated.';
 
@@ -23,7 +22,10 @@ assert.doesNotThrow(testMakeStatsCallback());
 
 function invalidCallbackThrowsTests() {
   callbackThrowValues.forEach((value) => {
-    assert.throws(testMakeStatsCallback(value), cbTypeError);
+    common.expectsError(testMakeStatsCallback(value), {
+      code: 'ERR_INVALID_CALLBACK',
+      type: TypeError
+    });
   });
 }
 

diff --git a/test/parallel/test-fs-mkdtemp-prefix-check.js b/test/parallel/test-fs-mkdtemp-prefix-check.js
@@ -1,22 +1,29 @@
 'use strict';
 const common = require('../common');
-const assert = require('assert');
 const fs = require('fs');
 
-const expectedError = /^TypeError: filename prefix is required$/;
 const prefixValues = [undefined, null, 0, true, false, 1, ''];
 
 function fail(value) {
-  assert.throws(
-    () => fs.mkdtempSync(value, {}),
-    expectedError
-  );
+  common.expectsError(
+    () => {
+      fs.mkdtempSync(value, {});
+    },
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError
+    });
 }
 
 function failAsync(value) {
-  assert.throws(
-    () => fs.mkdtemp(value, common.mustNotCall()), expectedError
-  );
+  common.expectsError(
+    () => {
+      fs.mkdtemp(value, common.mustNotCall());
+    },
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError
+    });
 }
 
 prefixValues.forEach((prefixValue) => {

diff --git a/test/parallel/test-fs-non-number-arguments-throw.js b/test/parallel/test-fs-non-number-arguments-throw.js
@@ -13,20 +13,32 @@ fs.writeFileSync(tempFile, 'abc\ndef');
 const sanity = 'def';
 const saneEmitter = fs.createReadStream(tempFile, { start: 4, end: 6 });
 
-assert.throws(function() {
-  fs.createReadStream(tempFile, { start: '4', end: 6 });
-}, /^TypeError: "start" option must be a Number$/,
-              "start as string didn't throw an error for createReadStream");
+common.expectsError(
+  () => {
+    fs.createReadStream(tempFile, { start: '4', end: 6 });
+  },
+  {
+    code: 'ERR_INVALID_ARG_TYPE',
+    type: TypeError
+  });
 
-assert.throws(function() {
-  fs.createReadStream(tempFile, { start: 4, end: '6' });
-}, /^TypeError: "end" option must be a Number$/,
-              "end as string didn't throw an error for createReadStream");
+common.expectsError(
+  () => {
+    fs.createReadStream(tempFile, { start: 4, end: '6' });
+  },
+  {
+    code: 'ERR_INVALID_ARG_TYPE',
+    type: TypeError
+  });
 
-assert.throws(function() {
-  fs.createWriteStream(tempFile, { start: '4' });
-}, /^TypeError: "start" option must be a Number$/,
-              "start as string didn't throw an error for createWriteStream");
+common.expectsError(
+  () => {
+    fs.createWriteStream(tempFile, { start: '4' });
+  },
+  {
+    code: 'ERR_INVALID_ARG_TYPE',
+    type: TypeError
+  });
 
 saneEmitter.on('data', common.mustCall(function(data) {
   assert.strictEqual(

diff --git a/test/parallel/test-fs-null-bytes.js b/test/parallel/test-fs-null-bytes.js
@@ -26,17 +26,27 @@ const fs = require('fs');
 const URL = require('url').URL;
 
 function check(async, sync) {
-  const expected = /Path must be a string without null bytes/;
   const argsSync = Array.prototype.slice.call(arguments, 2);
   const argsAsync = argsSync.concat((er) => {
-    assert(er && expected.test(er.message));
-    assert.strictEqual(er.code, 'ENOENT');
+    common.expectsError(
+      () => {
+        throw er;
+      },
+      {
+        code: 'ERR_INVALID_ARG_TYPE',
+        type: Error
+      });
   });
 
   if (sync) {
-    assert.throws(() => {
-      sync.apply(null, argsSync);
-    }, expected);
+    common.expectsError(
+      () => {
+        sync.apply(null, argsSync);
+      },
+      {
+        code: 'ERR_INVALID_ARG_TYPE',
+        type: Error,
+      });
   }
 
   if (async) {