buffer,errors: improve bigint, big numbers and more

This improves the error message from `ERR_OUT_OF_RANGE` by closer inspecting the value and logging numbers above 2 ** 32 by adding commas to the output for integer and bigint. BigInt is now also marked if used. Buffer errors also format the range as 2 ** n instead of showing a huge number. PR-URL: #27228 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
nodejs · May 1, 2019 · cac584d · cac584d
1 parent cb97de7
commit cac584d
Show file tree

Hide file tree

Showing 8 changed files with 65 additions and 13 deletions.
diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js
@@ -43,7 +43,20 @@ function checkBounds(buf, offset, byteLength) {
 
 function checkInt(value, min, max, buf, offset, byteLength) {
   if (value > max || value < min) {
-    throw new ERR_OUT_OF_RANGE('value', `>= ${min} and <= ${max}`, value);
+    // eslint-disable-next-line valid-typeof
+    const n = typeof min === 'bigint' ? 'n' : '';
+    let range;
+    if (byteLength > 3) {
+      if (min === 0 || min === 0n) {
+        range = `>= 0${n} and < 2${n} ** ${(byteLength + 1) * 8}${n}`;
+      } else {
+        range = `>= -(2${n} ** ${(byteLength + 1) * 8 - 1}${n}) and < 2 ** ` +
+                `${(byteLength + 1) * 8 - 1}${n}`;
+      }
+    } else {
+      range = `>= ${min}${n} and <= ${max}${n}`;
+    }
+    throw new ERR_OUT_OF_RANGE('value', range, value);
   }
   checkBounds(buf, offset, byteLength);
 }

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -10,7 +10,7 @@
 // value statically and permanently identifies the error. While the error
 // message may change, the code should not.
 
-const { Object } = primordials;
+const { Object, Math } = primordials;
 
 const kCode = Symbol('code');
 const kInfo = Symbol('info');
@@ -574,6 +574,17 @@ function oneOf(expected, thing) {
   }
 }
 
+// Only use this for integers! Decimal numbers do not work with this function.
+function addNumericalSeparator(val) {
+  let res = '';
+  let i = val.length;
+  const start = val[0] === '-' ? 1 : 0;
+  for (; i >= start + 4; i -= 3) {
+    res = `_${val.slice(i - 3, i)}${res}`;
+  }
+  return `${val.slice(0, i)}${res}`;
+}
+
 module.exports = {
   addCodeToName, // Exported for NghttpError
   codes,
@@ -990,7 +1001,20 @@ E('ERR_OUT_OF_RANGE',
     assert(range, 'Missing "range" argument');
     let msg = replaceDefaultBoolean ? str :
       `The value of "${str}" is out of range.`;
-    msg += ` It must be ${range}. Received ${input}`;
+    let received;
+    if (Number.isInteger(input) && Math.abs(input) > 2 ** 32) {
+      received = addNumericalSeparator(String(input));
+    // eslint-disable-next-line valid-typeof
+    } else if (typeof input === 'bigint') {
+      received = String(input);
+      if (input > 2n ** 32n || input < -(2n ** 32n)) {
+        received = addNumericalSeparator(received);
+      }
+      received += 'n';
+    } else {
+      received = lazyInternalUtilInspect().inspect(input);
+    }
+    msg += ` It must be ${range}. Received ${received}`;
     return msg;
   }, RangeError);
 E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s', Error);

diff --git a/test/parallel/test-buffer-bigint64.js b/test/parallel/test-buffer-bigint64.js
@@ -37,7 +37,11 @@ const buf = Buffer.allocUnsafe(8);
   assert.throws(function() {
     const val = 0x10000000000000000n;
     buf['writeBigUInt64' + endianness](val, 0);
-  }, RangeError);
+  }, {
+    code: 'ERR_OUT_OF_RANGE',
+    message: 'The value of "value" is out of range. It must be ' +
+             '>= 0n and < 2n ** 64n. Received 18_446_744_073_709_551_616n'
+  });
 
   // Should throw a TypeError upon invalid input
   assert.throws(function() {

diff --git a/test/parallel/test-buffer-writeint.js b/test/parallel/test-buffer-writeint.js
@@ -217,15 +217,21 @@ const errorOutOfBounds = common.expectsError({
     ['writeIntBE', 'writeIntLE'].forEach((fn) => {
       const min = -(2 ** (i * 8 - 1));
       const max = 2 ** (i * 8 - 1) - 1;
-
+      let range = `>= ${min} and <= ${max}`;
+      if (i > 4) {
+        range = `>= -(2 ** ${i * 8 - 1}) and < 2 ** ${i * 8 - 1}`;
+      }
       [min - 1, max + 1].forEach((val) => {
+        const received = i > 4 ?
+          String(val).replace(/(\d)(?=(\d\d\d)+(?!\d))/g, '$1_') :
+          val;
         assert.throws(() => {
           data[fn](val, 0, i);
         }, {
           code: 'ERR_OUT_OF_RANGE',
           name: 'RangeError',
           message: 'The value of "value" is out of range. ' +
-                   `It must be >= ${min} and <= ${max}. Received ${val}`
+                   `It must be ${range}. Received ${received}`
         });
       });
 

diff --git a/test/parallel/test-buffer-writeuint.js b/test/parallel/test-buffer-writeuint.js
@@ -171,14 +171,18 @@ const assert = require('assert');
 
   // Test 1 to 6 bytes.
   for (let i = 1; i < 6; i++) {
+    const range = i < 5 ? `= ${val - 1}` : ` 2 ** ${i * 8}`;
+    const received = i > 4 ?
+      String(val).replace(/(\d)(?=(\d\d\d)+(?!\d))/g, '$1_') :
+      val;
     ['writeUIntBE', 'writeUIntLE'].forEach((fn) => {
       assert.throws(() => {
         data[fn](val, 0, i);
       }, {
         code: 'ERR_OUT_OF_RANGE',
         name: 'RangeError',
         message: 'The value of "value" is out of range. ' +
-                 `It must be >= 0 and <= ${val - 1}. Received ${val}`
+                 `It must be >= 0 and <${range}. Received ${received}`
       });
 
       ['', '0', null, {}, [], () => {}, true, false].forEach((o) => {

diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js
@@ -111,8 +111,8 @@ assert.throws(
     }, {
       code: 'ERR_OUT_OF_RANGE',
       name: 'RangeError',
-      message: 'The value of "keylen" is out of range. It ' +
-               `must be >= 0 && < 4294967296. Received ${input}`
+      message: 'The value of "keylen" is out of range. It must be >= 0 && < ' +
+               `4294967296. Received ${input === -1 ? '-1' : '4_294_967_297'}`
     });
 });
 

diff --git a/test/parallel/test-event-emitter-max-listeners.js b/test/parallel/test-event-emitter-max-listeners.js
@@ -22,6 +22,7 @@
 'use strict';
 const common = require('../common');
 const events = require('events');
+const { inspect } = require('util');
 const e = new events.EventEmitter();
 
 e.on('maxListeners', common.mustCall());
@@ -38,7 +39,7 @@ for (const obj of throwsObjs) {
       code: 'ERR_OUT_OF_RANGE',
       type: RangeError,
       message: 'The value of "n" is out of range. ' +
-               `It must be a non-negative number. Received ${obj}`
+               `It must be a non-negative number. Received ${inspect(obj)}`
     }
   );
 
@@ -48,7 +49,7 @@ for (const obj of throwsObjs) {
       code: 'ERR_OUT_OF_RANGE',
       type: RangeError,
       message: 'The value of "defaultMaxListeners" is out of range. ' +
-               `It must be a non-negative number. Received ${obj}`
+               `It must be a non-negative number. Received ${inspect(obj)}`
     }
   );
 }

diff --git a/test/parallel/test-file-write-stream3.js b/test/parallel/test-file-write-stream3.js
@@ -199,8 +199,8 @@ const run_test_5 = common.mustCall(function() {
   };
   const err = {
     code: 'ERR_OUT_OF_RANGE',
-    message: 'The value of "start" is out of range. ' +
-             'It must be >= 0 and <= 2 ** 53 - 1. Received 9007199254740992',
+    message: 'The value of "start" is out of range. It must be ' +
+             '>= 0 and <= 2 ** 53 - 1. Received 9_007_199_254_740_992',
     type: RangeError
   };
   common.expectsError(fn, err);