2 batches of comments

Author: refack

doc/api/util.md

@@ -41,19 +41,22 @@ callbackFunction((err, ret) => {
 });
 ```
 
-will print something like:
+Will print something like:
 
 ```txt
 hello world
 ```
 
 *Note*:
+
 * Like with most callback style functions, the callback is executed in an
-async context (having a limited stacktrace), and if the callback throws the
-process will emit an `uncaughtException` event, and if not handled, will exit.
+async context (i.e. having a limited stacktrace). If the callback throws, the
+process will emit an `uncaughtException` event, and if not handled will exit.
+
 * Since `null` has a special meaning as the first argument to a callback, if a
-wrapped function rejects a `Promise` with a "falsy" value as a reason, we wrap
-the value in an `Error` with that value stored in a field named `cause`.
+wrapped function rejects a `Promise` with a falsy value as a reason, the value
+is wrapped in an `Error` with the original value stored in a field named
+`reason`.
 
 ```js
 function fn() {
@@ -62,12 +65,12 @@ function fn() {
 const callbackFunction = util.callbackify(fn);
 
 callbackFunction((err, ret) => {
-  err && err.cause && err.cause === null;  // true
-  err && err.message === 'Error: null';    // true
+  // When the Promise was rejected with `null` it is wrapped with an Error and
+  // the original value is stored in `reason`.
+  err && ('reason' in err) && err.reason === null;  // true
 });
 ```
 
-
 ## util.debuglog(section)
 <!-- YAML
 added: v0.11.3

lib/internal/errors.js

@@ -164,6 +164,7 @@ E('ERR_SOCKET_BAD_PORT', 'Port should be > 0 and < 65536');
 E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running');
 E('ERR_V8BREAKITERATOR', 'full ICU data not installed. ' +
   'See https://github.com/nodejs/node/wiki/Intl');
+E('NULL_REJECTION', 'Promise was rejected with "null"');
 // Add new errors from here...
 
 function invalidArgType(name, expected, actual) {

lib/util.js

@@ -1052,9 +1052,10 @@ function callbackifyOnRejected(reason, cb) {
   // occurred", we error-wrap so the callback consumer can distinguish between
   // "the promise rejected with null" or "the promise fulfilled with undefined".
   if (!reason) {
-    const newRej = new Error(String(reason));
-    newRej.cause = reason;
-    reason = newRej;
+    const newReason = new errors.Error('NULL_REJECTION');
+    newReason.reason = reason;
+    reason = newReason;
+    Error.captureStackTrace(reason, callbackifyOnRejected);
   }
   return cb(reason);
 }

test/fixtures/uncaught-exceptions/callbackify1.js

@@ -1,6 +1,6 @@
 'use strict';
 
-// Used to test `uncaughtException` is emitted
+// Used to test that `uncaughtException` is emitted
 
 const { callbackify } = require('util');
 

test/fixtures/uncaught-exceptions/callbackify2.js

@@ -1,6 +1,6 @@
 'use strict';
 
-// Used to test arguments to `uncaughtException` from a callback
+// Used to test the `uncaughtException` err object
 
 const assert = require('assert');
 const { callbackify } = require('util');

test/parallel/test-util-callbackify.js

@@ -1,12 +1,15 @@
 'use strict';
 const common = require('../common');
 
+// This test checks that the semantics of `util.callbackify` are as described in
+// the API docs
+
 const assert = require('assert');
 const { callbackify } = require('util');
 const { join } = require('path');
 const { execFile } = require('child_process');
 const fixtureDir = join(common.fixturesDir, 'uncaught-exceptions');
-const sentinalValues = [
+const sentinelValues = [
   'hello world',
   null,
   undefined,
@@ -18,8 +21,8 @@ const sentinalValues = [
 ];
 
 {
-  // Just works
-  for (const sentinel of sentinalValues) {
+  // Test that the resolution value is passed as second argument to callback
+  for (const sentinel of sentinelValues) {
     async function asyncFn() {
       return await Promise.resolve(sentinel);
     }
@@ -45,8 +48,8 @@ const sentinalValues = [
 }
 
 {
-  // Rejections
-  for (const sentinel of sentinalValues) {
+  // Test that rejection reason is passed as first argument to callback
+  for (const sentinel of sentinelValues) {
     async function asyncFn() {
       return await Promise.reject(sentinel);
     }
@@ -56,16 +59,19 @@ const sentinalValues = [
     cbAsyncFn(common.mustCall((err, ret) => {
       assert.strictEqual(ret, undefined);
       if (err instanceof Error) {
-        assert.strictEqual(String(sentinel).endsWith(err.message), true);
-        if ('cause' in err)
-          assert.strictEqual(err.cause, sentinel);
+        if ('reason' in err) {
+          assert.strictEqual(err.code, 'NULL_REJECTION');
+          assert.strictEqual(err.reason, sentinel);
+        } else {
+          assert.strictEqual(String(sentinel).endsWith(err.message), true);
+        }
       } else {
         assert.strictEqual(err, sentinel);
       }
     }));
   }
 
-  for (const sentinel of sentinalValues) {
+  for (const sentinel of sentinelValues) {
     function promiseFn() {
       return Promise.reject(sentinel);
     }
@@ -75,9 +81,12 @@ const sentinalValues = [
     cbPromiseFn(common.mustCall((err, ret) => {
       assert.strictEqual(ret, undefined);
       if (err instanceof Error) {
-        assert.strictEqual(String(sentinel).endsWith(err.message), true);
-        if ('cause' in err)
-          assert.strictEqual(err.cause, sentinel);
+        if ('reason' in err) {
+          assert.strictEqual(err.code, 'NULL_REJECTION');
+          assert.strictEqual(err.reason, sentinel);
+        } else {
+          assert.strictEqual(String(sentinel).endsWith(err.message), true);
+        }
       } else {
         assert.strictEqual(err, sentinel);
       }
@@ -86,8 +95,8 @@ const sentinalValues = [
 }
 
 {
-  // 3. args work
-  for (const sentinel of sentinalValues) {
+  // Test that arguments passed to callbackified function are passed to original
+  for (const sentinel of sentinelValues) {
     async function asyncFn(arg) {
       assert.strictEqual(arg, sentinel);
       return await Promise.resolve(arg);
@@ -115,8 +124,8 @@ const sentinalValues = [
 }
 
 {
-  // `this` binding
-  for (const sentinel of sentinalValues) {
+  // Test that `this` binding is the same for callbackified and original
+  for (const sentinel of sentinelValues) {
     const iAmThis = {
       fn(arg) {
         assert.strictEqual(this, iAmThis);
@@ -148,7 +157,7 @@ const sentinalValues = [
 }
 
 {
-  // `uncaughtException` aborts process
+  // Test that callback that throws emits an `uncaughtException` event
   const fixture = join(fixtureDir, 'callbackify1.js');
   execFile(
     process.argv[0],
@@ -165,7 +174,7 @@ const sentinalValues = [
 }
 
 {
-  // handled `uncaughtException` works and passes rejection reason
+  // Test that handled `uncaughtException` works and passes rejection reason
   const fixture = join(fixtureDir, 'callbackify2.js');
   execFile(
     process.argv[0],