Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 87fb1c2

Browse filesBrowse files
BridgeARBridgeAR
committed
errors: make sure all Node.js errors show their properties
This improves Node.js errors by always showing the attached properties when inspecting such an error. This applies especially to SystemError. It did often not show any properties but now all properties will be visible. This is done in a mainly backwards compatible way. Instead of using prototype getters and setters, the property is now set directly on the error. PR-URL: #29677 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent bf1727a commit 87fb1c2
Copy full SHA for 87fb1c2

File tree

Expand file treeCollapse file tree

6 files changed

+103
-85
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+103
-85
lines changed
Open diff view settings
Collapse file

‎lib/internal/errors.js‎

Copy file name to clipboardExpand all lines: lib/internal/errors.js
+67-72Lines changed: 67 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212

1313
const { Object, Math } = primordials;
1414

15-
const kCode = Symbol('code');
16-
const kInfo = Symbol('info');
1715
const messages = new Map();
1816
const codes = {};
1917

@@ -121,76 +119,86 @@ class SystemError extends Error {
121119
writable: true,
122120
configurable: true
123121
});
124-
Object.defineProperty(this, kInfo, {
125-
configurable: false,
126-
enumerable: false,
127-
value: context
128-
});
129-
Object.defineProperty(this, kCode, {
130-
configurable: true,
131-
enumerable: false,
132-
value: key,
133-
writable: true
134-
});
135122
addCodeToName(this, 'SystemError', key);
136-
}
137123

138-
get code() {
139-
return this[kCode];
140-
}
124+
this.code = key;
141125

142-
set code(value) {
143-
Object.defineProperty(this, 'code', {
144-
configurable: true,
126+
Object.defineProperty(this, 'info', {
127+
value: context,
145128
enumerable: true,
146-
value,
147-
writable: true
129+
configurable: true,
130+
writable: false
148131
});
149-
}
150-
151-
get info() {
152-
return this[kInfo];
153-
}
154132

155-
get errno() {
156-
return this[kInfo].errno;
157-
}
158-
159-
set errno(val) {
160-
this[kInfo].errno = val;
161-
}
162-
163-
get syscall() {
164-
return this[kInfo].syscall;
165-
}
166-
167-
set syscall(val) {
168-
this[kInfo].syscall = val;
169-
}
170-
171-
get path() {
172-
return this[kInfo].path !== undefined ?
173-
this[kInfo].path.toString() : undefined;
174-
}
133+
Object.defineProperty(this, 'errno', {
134+
get() {
135+
return context.errno;
136+
},
137+
set: (value) => {
138+
context.errno = value;
139+
},
140+
enumerable: true,
141+
configurable: true
142+
});
175143

176-
set path(val) {
177-
this[kInfo].path = val ?
178-
lazyBuffer().from(val.toString()) : undefined;
179-
}
144+
Object.defineProperty(this, 'syscall', {
145+
get() {
146+
return context.syscall;
147+
},
148+
set: (value) => {
149+
context.syscall = value;
150+
},
151+
enumerable: true,
152+
configurable: true
153+
});
180154

181-
get dest() {
182-
return this[kInfo].path !== undefined ?
183-
this[kInfo].dest.toString() : undefined;
184-
}
155+
if (context.path !== undefined) {
156+
// TODO(BridgeAR): Investigate why and when the `.toString()` was
157+
// introduced. The `path` and `dest` properties in the context seem to
158+
// always be of type string. We should probably just remove the
159+
// `.toString()` and `Buffer.from()` operations and set the value on the
160+
// context as the user did.
161+
Object.defineProperty(this, 'path', {
162+
get() {
163+
return context.path != null ?
164+
context.path.toString() : context.path;
165+
},
166+
set: (value) => {
167+
context.path = value ?
168+
lazyBuffer().from(value.toString()) : undefined;
169+
},
170+
enumerable: true,
171+
configurable: true
172+
});
173+
}
185174

186-
set dest(val) {
187-
this[kInfo].dest = val ?
188-
lazyBuffer().from(val.toString()) : undefined;
175+
if (context.dest !== undefined) {
176+
Object.defineProperty(this, 'dest', {
177+
get() {
178+
return context.dest != null ?
179+
context.dest.toString() : context.dest;
180+
},
181+
set: (value) => {
182+
context.dest = value ?
183+
lazyBuffer().from(value.toString()) : undefined;
184+
},
185+
enumerable: true,
186+
configurable: true
187+
});
188+
}
189189
}
190190

191191
toString() {
192192
return `${this.name} [${this.code}]: ${this.message}`;
193193
}
194+
195+
[Symbol.for('nodejs.util.inspect.custom')](recurseTimes, ctx) {
196+
return lazyInternalUtilInspect().inspect(this, {
197+
...ctx,
198+
getters: true,
199+
customInspect: false
200+
});
201+
}
194202
}
195203

196204
function makeSystemErrorWithCode(key) {
@@ -221,19 +229,7 @@ function makeNodeErrorWithCode(Base, key) {
221229
configurable: true
222230
});
223231
addCodeToName(this, super.name, key);
224-
}
225-
226-
get code() {
227-
return key;
228-
}
229-
230-
set code(value) {
231-
Object.defineProperty(this, 'code', {
232-
configurable: true,
233-
enumerable: true,
234-
value,
235-
writable: true
236-
});
232+
this.code = key;
237233
}
238234

239235
toString() {
@@ -394,7 +390,6 @@ function uvException(ctx) {
394390
err[prop] = ctx[prop];
395391
}
396392

397-
// TODO(BridgeAR): Show the `code` property as part of the stack.
398393
err.code = code;
399394
if (path) {
400395
err.path = path;
Collapse file

‎test/message/internal_assert.out‎

Copy file name to clipboardExpand all lines: test/message/internal_assert.out
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,6 @@ Please open an issue with this stack trace at https://github.com/nodejs/node/iss
1212
at *
1313
at *
1414
at *
15-
at *
15+
at * {
16+
code: 'ERR_INTERNAL_ASSERTION'
17+
}
Collapse file

‎test/message/internal_assert_fail.out‎

Copy file name to clipboardExpand all lines: test/message/internal_assert_fail.out
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,6 @@ Please open an issue with this stack trace at https://github.com/nodejs/node/iss
1313
at *
1414
at *
1515
at *
16-
at *
16+
at * {
17+
code: 'ERR_INTERNAL_ASSERTION'
18+
}
Collapse file

‎test/parallel/test-dgram-socket-buffer-size.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-dgram-socket-buffer-size.js
+20-3Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
const common = require('../common');
55
const assert = require('assert');
66
const dgram = require('dgram');
7+
const { inspect } = require('util');
78
const { SystemError } = require('internal/errors');
89
const { internalBinding } = require('internal/test/binding');
910
const {
@@ -22,7 +23,7 @@ function getExpectedError(type) {
2223
'ENOTSOCK (socket operation on non-socket)' : 'EBADF (bad file descriptor)';
2324
const error = {
2425
code: 'ERR_SOCKET_BUFFER_SIZE',
25-
type: SystemError,
26+
name: 'SystemError',
2627
message: `Could not get or set buffer size: ${syscall} returned ${suffix}`,
2728
info: {
2829
code,
@@ -40,9 +41,25 @@ function getExpectedError(type) {
4041

4142
const socket = dgram.createSocket('udp4');
4243

43-
common.expectsError(() => {
44+
assert.throws(() => {
4445
socket.setSendBufferSize(8192);
45-
}, errorObj);
46+
}, (err) => {
47+
assert.strictEqual(
48+
inspect(err).replace(/^ +at .*\n/gm, ''),
49+
`SystemError [ERR_SOCKET_BUFFER_SIZE]: ${errorObj.message}\n` +
50+
" code: 'ERR_SOCKET_BUFFER_SIZE',\n" +
51+
' info: {\n' +
52+
` errno: ${errorObj.info.errno},\n` +
53+
` code: '${errorObj.info.code}',\n` +
54+
` message: '${errorObj.info.message}',\n` +
55+
` syscall: '${errorObj.info.syscall}'\n` +
56+
' },\n' +
57+
` errno: [Getter/Setter: ${errorObj.info.errno}],\n` +
58+
` syscall: [Getter/Setter: '${errorObj.info.syscall}']\n` +
59+
'}'
60+
);
61+
return true;
62+
});
4663

4764
common.expectsError(() => {
4865
socket.getSendBufferSize();
Collapse file

‎test/parallel/test-internal-errors.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-internal-errors.js
+6-6Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ common.expectsError(() => {
8282
{
8383
const myError = new errors.codes.TEST_ERROR_1('foo');
8484
assert.strictEqual(myError.code, 'TEST_ERROR_1');
85-
assert.strictEqual(myError.hasOwnProperty('code'), false);
85+
assert.strictEqual(myError.hasOwnProperty('code'), true);
8686
assert.strictEqual(myError.hasOwnProperty('name'), false);
87-
assert.deepStrictEqual(Object.keys(myError), []);
87+
assert.deepStrictEqual(Object.keys(myError), ['code']);
8888
const initialName = myError.name;
8989
myError.code = 'FHQWHGADS';
9090
assert.strictEqual(myError.code, 'FHQWHGADS');
@@ -99,11 +99,11 @@ common.expectsError(() => {
9999
// browser. Note that `name` becomes enumerable after being assigned.
100100
{
101101
const myError = new errors.codes.TEST_ERROR_1('foo');
102-
assert.deepStrictEqual(Object.keys(myError), []);
102+
assert.deepStrictEqual(Object.keys(myError), ['code']);
103103
const initialToString = myError.toString();
104104

105105
myError.name = 'Fhqwhgads';
106-
assert.deepStrictEqual(Object.keys(myError), ['name']);
106+
assert.deepStrictEqual(Object.keys(myError), ['code', 'name']);
107107
assert.notStrictEqual(myError.toString(), initialToString);
108108
}
109109

@@ -114,7 +114,7 @@ common.expectsError(() => {
114114
let initialConsoleLog = '';
115115
hijackStdout((data) => { initialConsoleLog += data; });
116116
const myError = new errors.codes.TEST_ERROR_1('foo');
117-
assert.deepStrictEqual(Object.keys(myError), []);
117+
assert.deepStrictEqual(Object.keys(myError), ['code']);
118118
const initialToString = myError.toString();
119119
console.log(myError);
120120
assert.notStrictEqual(initialConsoleLog, '');
@@ -124,7 +124,7 @@ common.expectsError(() => {
124124
let subsequentConsoleLog = '';
125125
hijackStdout((data) => { subsequentConsoleLog += data; });
126126
myError.message = 'Fhqwhgads';
127-
assert.deepStrictEqual(Object.keys(myError), []);
127+
assert.deepStrictEqual(Object.keys(myError), ['code']);
128128
assert.notStrictEqual(myError.toString(), initialToString);
129129
console.log(myError);
130130
assert.strictEqual(subsequentConsoleLog, initialConsoleLog);
Collapse file

‎test/parallel/test-repl-top-level-await.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-repl-top-level-await.js
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,10 @@ async function ctrlCTest() {
161161
]), [
162162
'await timeout(100000)\r',
163163
'Thrown:',
164-
'Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' +
165-
'Script execution was interrupted by `SIGINT`',
164+
'[Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' +
165+
'Script execution was interrupted by `SIGINT`] {',
166+
" code: 'ERR_SCRIPT_EXECUTION_INTERRUPTED'",
167+
'}',
166168
PROMPT
167169
]);
168170
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.