lib, test: add duplicate symbol checking in E()

Add duplicate symbol checking in E() to avoid potential confusing result. Increase coverage of internal/errors. PR-URL: #11829 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
nodejs · Mar 20, 2017 · 77c69f7 · 77c69f7
1 parent 7e23072
commit 77c69f7
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 0 deletions.
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -60,6 +60,8 @@ function message(key, args) {
 // Utility function for registering the error codes. Only used here. Exported
 // *only* to allow for testing.
 function E(sym, val) {
+  const assert = lazyAssert();
+  assert(messages.has(sym) === false, `Error symbol: ${sym} was already used.`);
   messages.set(sym, typeof val === 'function' ? val : String(val));
 }
 

diff --git a/test/parallel/test-internal-errors.js b/test/parallel/test-internal-errors.js
@@ -12,6 +12,7 @@ const err1 = new errors.Error('TEST_ERROR_1', 'test');
 const err2 = new errors.TypeError('TEST_ERROR_1', 'test');
 const err3 = new errors.RangeError('TEST_ERROR_1', 'test');
 const err4 = new errors.Error('TEST_ERROR_2', 'abc', 'xyz');
+const err5 = new errors.Error('TEST_ERROR_1');
 
 assert(err1 instanceof Error);
 assert.strictEqual(err1.name, 'Error[TEST_ERROR_1]');
@@ -33,6 +34,11 @@ assert.strictEqual(err4.name, 'Error[TEST_ERROR_2]');
 assert.strictEqual(err4.message, 'abc xyz');
 assert.strictEqual(err4.code, 'TEST_ERROR_2');
 
+assert(err5 instanceof Error);
+assert.strictEqual(err5.name, 'Error[TEST_ERROR_1]');
+assert.strictEqual(err5.message, 'Error for testing purposes: %s');
+assert.strictEqual(err5.code, 'TEST_ERROR_1');
+
 assert.throws(
   () => new errors.Error('TEST_FOO_KEY'),
   /^AssertionError: An invalid error message key was used: TEST_FOO_KEY.$/);
@@ -118,3 +124,9 @@ assert.throws(() => {
                            type: TypeError,
                            message: /^Error for testing 2/ }));
 }, /AssertionError: .+ does not match \S/);
+
+assert.doesNotThrow(() => errors.E('TEST_ERROR_USED_SYMBOL'));
+assert.throws(
+  () => errors.E('TEST_ERROR_USED_SYMBOL'),
+  /^AssertionError: Error symbol: TEST_ERROR_USED_SYMBOL was already used\.$/
+);