Skip to content

Commit f012802

Browse files
joyeecheungMylesBorins
authored andcommitted
util: implement util.getSystemErrorName()
Reimplement uv.errname() as internal/util.getSystemErrorName() to avoid the memory leaks caused by unknown error codes and avoid calling into C++ for the error names. Also expose it as a public API for external use. Backport-PR-URL: #18916 PR-URL: #18186 Refs: http://docs.libuv.org/en/v1.x/errors.html#c.uv_err_name Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
1 parent a92a359 commit f012802

9 files changed

+60
-23
lines changed

doc/api/util.md

+21-1
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,25 @@ without any formatting.
224224
util.format('%% %s'); // '%% %s'
225225
```
226226

227+
## util.getSystemErrorName(err)
228+
<!-- YAML
229+
added: REPLACEME
230+
-->
231+
232+
* `err` {number}
233+
* Returns: {string}
234+
235+
Returns the string name for a numeric error code that comes from a Node.js API.
236+
The mapping between error codes and error names is platform-dependent.
237+
See [Common System Errors][] for the names of common errors.
238+
239+
```js
240+
fs.access('file/that/does/not/exist', (err) => {
241+
const name = util.getSystemErrorName(err.errno);
242+
console.error(name); // ENOENT
243+
});
244+
```
245+
227246
## util.inherits(constructor, superConstructor)
228247
<!-- YAML
229248
added: v0.3.0
@@ -1222,5 +1241,6 @@ Deprecated predecessor of `console.log`.
12221241
[Customizing `util.inspect` colors]: #util_customizing_util_inspect_colors
12231242
[Internationalization]: intl.html
12241243
[WHATWG Encoding Standard]: https://encoding.spec.whatwg.org/
1225-
[constructor]: https://developer.mozilla.org/en-US/JavaScript/Reference/Global_Objects/Object/constructor
1244+
[Common System Errors]: errors.html#errors_common_system_errors
1245+
[constructor]: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Object/constructor
12261246
[semantically incompatible]: https://github.com/nodejs/node/issues/4179

lib/child_process.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@
2222
'use strict';
2323

2424
const util = require('util');
25-
const { deprecate, convertToValidSignal } = require('internal/util');
25+
const {
26+
deprecate, convertToValidSignal, getSystemErrorName
27+
} = require('internal/util');
2628
const { isUint8Array } = require('internal/util/types');
2729
const { createPromise,
2830
promiseResolve, promiseReject } = process.binding('util');
2931
const debug = util.debuglog('child_process');
3032
const { Buffer } = require('buffer');
3133
const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
32-
const { errname } = process.binding('uv');
3334
const child_process = require('internal/child_process');
3435
const {
3536
_validateStdio,
@@ -271,7 +272,7 @@ exports.execFile = function(file /*, args, options, callback*/) {
271272
if (!ex) {
272273
ex = new Error('Command failed: ' + cmd + '\n' + stderr);
273274
ex.killed = child.killed || killed;
274-
ex.code = code < 0 ? errname(code) : code;
275+
ex.code = code < 0 ? getSystemErrorName(code) : code;
275276
ex.signal = signal;
276277
}
277278

lib/internal/util.js

+12-1
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
const errors = require('internal/errors');
44
const binding = process.binding('util');
55
const { signals } = process.binding('constants').os;
6-
76
const { createPromise, promiseResolve, promiseReject } = binding;
7+
const { errmap } = process.binding('uv');
88

99
const kArrowMessagePrivateSymbolIndex = binding.arrow_message_private_symbol;
1010
const kDecoratedPrivateSymbolIndex = binding.decorated_private_symbol;
@@ -207,6 +207,16 @@ function getConstructorOf(obj) {
207207
return null;
208208
}
209209

210+
function getSystemErrorName(err) {
211+
if (typeof err !== 'number' || err >= 0 || !Number.isSafeInteger(err)) {
212+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'err',
213+
'negative number');
214+
}
215+
216+
const entry = errmap.get(err);
217+
return entry ? entry[0] : `Unknown system error ${err}`;
218+
}
219+
210220
const kCustomPromisifiedSymbol = Symbol('util.promisify.custom');
211221
const kCustomPromisifyArgsSymbol = Symbol('customPromisifyArgs');
212222

@@ -297,6 +307,7 @@ module.exports = {
297307
emitExperimentalWarning,
298308
filterDuplicateStrings,
299309
getConstructorOf,
310+
getSystemErrorName,
300311
isError,
301312
join,
302313
normalizeEncoding,

lib/util.js

+3-7
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ const errors = require('internal/errors');
2525
const { TextDecoder, TextEncoder } = require('internal/encoding');
2626
const { isBuffer } = require('buffer').Buffer;
2727

28-
const { errname } = process.binding('uv');
29-
3028
const {
3129
getPromiseDetails,
3230
getProxyDetails,
@@ -56,6 +54,7 @@ const {
5654
customInspectSymbol,
5755
deprecate,
5856
getConstructorOf,
57+
getSystemErrorName,
5958
isError,
6059
promisify,
6160
join,
@@ -992,11 +991,7 @@ function error(...args) {
992991
}
993992

994993
function _errnoException(err, syscall, original) {
995-
if (typeof err !== 'number' || err >= 0 || !Number.isSafeInteger(err)) {
996-
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'err',
997-
'negative number');
998-
}
999-
const name = errname(err);
994+
const name = getSystemErrorName(err);
1000995
var message = `${syscall} ${name}`;
1001996
if (original)
1002997
message += ` ${original}`;
@@ -1085,6 +1080,7 @@ module.exports = exports = {
10851080
debuglog,
10861081
deprecate,
10871082
format,
1083+
getSystemErrorName,
10881084
inherits,
10891085
inspect,
10901086
isArray: Array.isArray,

src/uv.cc

+2
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ using v8::String;
3939
using v8::Value;
4040

4141

42+
// TODO(joyeecheung): deprecate this function in favor of
43+
// lib/util.getSystemErrorName()
4244
void ErrName(const FunctionCallbackInfo<Value>& args) {
4345
Environment* env = Environment::GetCurrent(args);
4446
int err = args[0]->Int32Value();

test/parallel/test-child-process-execfile.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
'use strict';
2+
23
const common = require('../common');
34
const assert = require('assert');
45
const execFile = require('child_process').execFile;
5-
const uv = process.binding('uv');
6+
const { getSystemErrorName } = require('util');
67
const fixtures = require('../common/fixtures');
78

89
const fixture = fixtures.path('exit.js');
@@ -27,7 +28,7 @@ const execOpts = { encoding: 'utf8', shell: true };
2728
const code = -1;
2829
const callback = common.mustCall((err, stdout, stderr) => {
2930
assert.strictEqual(err.toString().trim(), errorString);
30-
assert.strictEqual(err.code, uv.errname(code));
31+
assert.strictEqual(err.code, getSystemErrorName(code));
3132
assert.strictEqual(err.killed, true);
3233
assert.strictEqual(err.signal, null);
3334
assert.strictEqual(err.cmd, process.execPath);

test/parallel/test-net-server-listen-handle.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const common = require('../common');
44
const assert = require('assert');
55
const net = require('net');
66
const fs = require('fs');
7-
const uv = process.binding('uv');
7+
const { getSystemErrorName } = require('util');
88
const { TCP, constants: TCPConstants } = process.binding('tcp_wrap');
99
const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
1010

@@ -47,9 +47,10 @@ function randomHandle(type) {
4747
handleName = `pipe ${path}`;
4848
}
4949

50-
if (errno < 0) { // uv.errname requires err < 0
51-
assert(errno >= 0, `unable to bind ${handleName}: ${uv.errname(errno)}`);
50+
if (errno < 0) {
51+
assert.fail(`unable to bind ${handleName}: ${getSystemErrorName(errno)}`);
5252
}
53+
5354
if (!common.isWindows) { // fd doesn't work on windows
5455
// err >= 0 but fd = -1, should not happen
5556
assert.notStrictEqual(handle.fd, -1,

test/parallel/test-uv-errno.js

+9-5
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,31 @@
22

33
const common = require('../common');
44
const assert = require('assert');
5-
const util = require('util');
6-
const uv = process.binding('uv');
5+
const {
6+
getSystemErrorName,
7+
_errnoException
8+
} = require('util');
79

10+
const uv = process.binding('uv');
811
const keys = Object.keys(uv);
912

1013
keys.forEach((key) => {
1114
if (!key.startsWith('UV_'))
1215
return;
1316

1417
assert.doesNotThrow(() => {
15-
const err = util._errnoException(uv[key], 'test');
18+
const err = _errnoException(uv[key], 'test');
1619
const name = uv.errname(uv[key]);
17-
assert.strictEqual(err.code, err.errno);
20+
assert.strictEqual(getSystemErrorName(uv[key]), name);
1821
assert.strictEqual(err.code, name);
22+
assert.strictEqual(err.code, err.errno);
1923
assert.strictEqual(err.message, `test ${name}`);
2024
});
2125
});
2226

2327
[0, 1, 'test', {}, [], Infinity, -Infinity, NaN].forEach((key) => {
2428
common.expectsError(
25-
() => util._errnoException(key),
29+
() => _errnoException(key),
2630
{
2731
code: 'ERR_INVALID_ARG_TYPE',
2832
type: TypeError,

test/sequential/test-async-wrap-getasyncid.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const net = require('net');
77
const providers = Object.assign({}, process.binding('async_wrap').Providers);
88
const fixtures = require('../common/fixtures');
99
const tmpdir = require('../common/tmpdir');
10+
const { getSystemErrorName } = require('util');
1011

1112
// Make sure that all Providers are tested.
1213
{
@@ -214,7 +215,7 @@ if (common.hasCrypto) { // eslint-disable-line crypto-check
214215
// Use a long string to make sure the write happens asynchronously.
215216
const err = handle.writeLatin1String(wreq, 'hi'.repeat(100000));
216217
if (err)
217-
throw new Error(`write failed: ${process.binding('uv').errname(err)}`);
218+
throw new Error(`write failed: ${getSystemErrorName(err)}`);
218219
testInitialized(wreq, 'WriteWrap');
219220
});
220221
req.address = common.localhostIPv4;

0 commit comments

Comments
 (0)