Skip to content

Commit 886835c

Browse files
refackrefack
committed
addressing comment + more tests
1 parent ac8ba26 commit 886835c

5 files changed

Lines changed: 109 additions & 48 deletions

File tree

‎doc/api/util.md‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,14 @@ const util = require('util');
1515
added: REPLACEME
1616
-->
1717

18-
* `original` {Function} An async function, or a Promise factory
19-
* Returns: {Function} a "callback style" function
18+
* `original` {Function} An `async` function
19+
* Returns: {Function} a callback style function
2020

21-
Takes an async function and returns a function following the common Node.js
22-
callback style, i.e. taking a `(err, val) => ...` callback as the last argument.
21+
Takes an `async` function (or a function that returns a Promise) and returns a
22+
function following the common Node.js callback style, i.e. taking a `(err, val)
23+
=> ...` callback as the last argument. In the callback, the first argument will
24+
be the rejection reason (or `null` if function call resolved successfully), and
25+
the second argument will be the resolved value.
2326

2427
For example:
2528

@@ -28,23 +31,23 @@ const util = require('util');
2831
async function fn() {
2932
return await Promise.resolve('hello world');
3033
}
31-
const cbFn = util.callbackify(fn);
34+
const callbackFunction = util.callbackify(fn);
3235

33-
cbFn((err, ret) => {
36+
callbackFunction((err, ret) => {
3437
if (err) throw err;
3538
console.log(ret);
3639
});
3740
```
3841

39-
will output something like:
42+
will print something like:
4043

4144
```txt
4245
hello world
4346
```
4447

45-
*Note*: Like with most "callback style" function, the callback is executed in an
46-
async context (having a limited stacktrace), and if the callback throws, the
47-
process will emit an `uncaughtException` event and if not handles will exit.
48+
*Note*: Like with most callback style functions, the callback is executed in an
49+
async context (having a limited stacktrace), and if the callback throws the
50+
process will emit an `uncaughtException` event, and if not handled, will exit.
4851

4952
## util.debuglog(section)
5053
<!-- YAML

‎lib/util.js‎

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,57 +1046,49 @@ process.versions[exports.inspect.custom] =
10461046

10471047
exports.promisify = internalUtil.promisify;
10481048

1049-
// Callbackify section
1050-
const CBFY_ERR = 'The last argument to the callbackified function must be a ' +
1051-
'callback style function';
1052-
1053-
10541049
function callbackifyOnRejected(reason, cb) {
1055-
// !reason guard inspired by BlueBird https://goo.gl/t5IS6M
1050+
// `!reason` guard inspired by bluebird (Ref: https://goo.gl/t5IS6M).
10561051
// Because `null` is a special error value in callbacks which means "no error
1057-
// occurred". We error-wrap so the callback consumer can distinguish between
1052+
// occurred", we error-wrap so the callback consumer can distinguish between
10581053
// "the promise rejected with null" or "the promise fulfilled with undefined".
10591054
if (!reason) {
1060-
const newRej = new Error(reason + '');
1055+
const newRej = new Error(String(reason));
10611056
newRej.cause = reason;
10621057
reason = newRej;
10631058
}
10641059
return cb(reason);
10651060
}
10661061

10671062

1068-
function assertLastArgIsFunction(args) {
1069-
const cb = args.pop();
1070-
if (typeof cb !== 'function') throw new TypeError(CBFY_ERR);
1071-
return cb;
1072-
}
1073-
1074-
1075-
/**
1076-
* @function callbackify
1077-
* @param {function(): Promise} asyncEndpoint
1078-
* @public
1079-
*/
1080-
function callbackify(asyncEndpoint) {
1081-
if (typeof asyncEndpoint !== 'function')
1082-
throw new TypeError('The argument to callbackify() must be a function');
1063+
function callbackify(original) {
1064+
if (typeof original !== 'function') {
1065+
throw new errors.TypeError(
1066+
'ERR_INVALID_ARG_TYPE',
1067+
'original',
1068+
'function');
1069+
}
10831070

10841071
// We DO NOT return the promise as it gives the user a false sense that
10851072
// the promise is actually somehow related to the callback's execution
10861073
// and that the callback throwing will reject the promise.
10871074
function callbackified(...args) {
1088-
const cb = assertLastArgIsFunction(args);
1089-
asyncEndpoint.call(this, ...args)
1075+
const cb = args.pop();
1076+
if (typeof cb !== 'function') {
1077+
throw new errors.TypeError(
1078+
'ERR_INVALID_ARG_TYPE',
1079+
'last argument',
1080+
'function');
1081+
}
1082+
original.call(this, ...args)
10901083
.then((ret) => process.nextTick(cb, null, ret),
10911084
(rej) => process.nextTick(callbackifyOnRejected, rej, cb)
10921085
);
10931086
}
10941087

1095-
Object.setPrototypeOf(callbackified, Object.getPrototypeOf(asyncEndpoint));
1088+
Object.setPrototypeOf(callbackified, Object.getPrototypeOf(original));
10961089
Object.defineProperties(callbackified,
1097-
Object.getOwnPropertyDescriptors(asyncEndpoint));
1090+
Object.getOwnPropertyDescriptors(original));
10981091
return callbackified;
10991092
}
11001093

11011094
exports.callbackify = callbackify;
1102-
// End Callbackify

‎test/fixtures/uncaught-exceptions/callbackify1.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
'use strict';
2+
3+
// Used to test `uncaughtException` is emitted
4+
5+
const { callbackify } = require('util');
6+
7+
{
8+
const sentinel = new Error(__filename);
9+
10+
async function fn3 () {
11+
return await Promise.reject(sentinel);
12+
}
13+
14+
const cbFn = callbackify(fn3);
15+
16+
cbFn((err, ret) => {
17+
throw err;
18+
});
19+
}

‎test/fixtures/uncaught-exceptions/callbackify2.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
'use strict';
2+
3+
// Used to test arguments to `uncaughtException` from a callback
4+
5+
const assert = require('assert');
6+
const { callbackify } = require('util');
7+
8+
{
9+
const sentinel = new Error(__filename);
10+
process.once('uncaughtException',(err) => {
11+
assert.strictEqual(err, sentinel);
12+
console.log(err.message);
13+
});
14+
async function fn3() {
15+
return await Promise.reject(sentinel);
16+
}
17+
const cbFn = callbackify(fn3);
18+
19+
cbFn((err, ret) => assert.ifError(err));
20+
}

‎test/parallel/test-util-callbackify.js‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ const common = require('../common');
44

55
const assert = require('assert');
66
const { callbackify } = require('util');
7-
7+
const { join } = require('path');
8+
const { execFile } = require('child_process');
9+
const fixtureDir = join(common.fixturesDir, 'uncaught-exceptions');
810

911
{
1012
const sentinel = 'hello world';
@@ -14,7 +16,7 @@ const { callbackify } = require('util');
1416
const cbFn = callbackify(fn);
1517

1618
cbFn(common.mustCall((err, ret) => {
17-
if (err) assert.ifError(err);
19+
assert.ifError(err);
1820
assert.strictEqual(ret, sentinel);
1921
}));
2022
}
@@ -27,23 +29,48 @@ const { callbackify } = require('util');
2729
const cbFn = callbackify(fn2);
2830

2931
cbFn(common.mustCall((err, ret) => {
30-
if (err) assert.ifError(err);
32+
assert.ifError(err);
3133
assert.strictEqual(ret, sentinel);
3234
}));
3335
}
3436

3537
{
36-
const sentinel = new Error('hello world 3');
37-
process.once('uncaughtException', common.mustCall((err) => {
38-
assert.strictEqual(err, sentinel);
39-
}));
4038
async function fn3() {
41-
return await Promise.reject(sentinel);
39+
return await Promise.reject(null);
4240
}
4341
const cbFn = callbackify(fn3);
4442

4543
cbFn(common.mustCall((err, ret) => {
46-
if (err) assert.ifError(err);
47-
common.mustNotCall('should not reach here')();
44+
assert.strictEqual(Object.getPrototypeOf(err).name, 'Error');
45+
assert.strictEqual(err.message, 'null');
4846
}));
4947
}
48+
49+
{
50+
const fixture = join(fixtureDir, 'callbackify1.js');
51+
execFile(
52+
process.argv[0],
53+
[fixture],
54+
common.mustCall((err, stdout, stderr) => {
55+
assert.strictEqual(err.code, 1);
56+
assert.strictEqual(Object.getPrototypeOf(err).name, 'Error');
57+
assert.strictEqual(stdout, '');
58+
const errLines = stderr.trim().split(/[\r\n]+/g);
59+
const errLine = errLines.find((l) => /^Error/.exec(l));
60+
assert.strictEqual(errLine, `Error: ${fixture}`);
61+
})
62+
);
63+
}
64+
65+
{
66+
const fixture = join(fixtureDir, 'callbackify2.js');
67+
execFile(
68+
process.argv[0],
69+
[fixture],
70+
common.mustCall((err, stdout, stderr) => {
71+
assert.ifError(err);
72+
assert.strictEqual(stdout.trim(), fixture);
73+
assert.strictEqual(stderr, '');
74+
})
75+
);
76+
}

0 commit comments

Comments
 (0)