Skip to content

Commit aa3b02b

Browse 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.
1 parent cbd3a1c commit aa3b02b

File tree

6 files changed

+98
-85
lines changed

6 files changed

+98
-85
lines changed

‎lib/internal/errors.js‎

Lines changed: 62 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

@@ -107,76 +105,81 @@ class SystemError extends Error {
107105
writable: true,
108106
configurable: true
109107
});
110-
Object.defineProperty(this, kInfo, {
111-
configurable: false,
112-
enumerable: false,
113-
value: context
114-
});
115-
Object.defineProperty(this, kCode, {
116-
configurable: true,
117-
enumerable: false,
118-
value: key,
119-
writable: true
120-
});
121108
addCodeToName(this, 'SystemError', key);
122-
}
123109

124-
get code() {
125-
return this[kCode];
126-
}
110+
this.code = key;
127111

128-
set code(value) {
129-
Object.defineProperty(this, 'code', {
130-
configurable: true,
112+
Object.defineProperty(this, 'info', {
113+
value: context,
131114
enumerable: true,
132-
value,
133-
writable: true
115+
configurable: true,
116+
writable: false
134117
});
135-
}
136-
137-
get info() {
138-
return this[kInfo];
139-
}
140118

141-
get errno() {
142-
return this[kInfo].errno;
143-
}
144-
145-
set errno(val) {
146-
this[kInfo].errno = val;
147-
}
148-
149-
get syscall() {
150-
return this[kInfo].syscall;
151-
}
152-
153-
set syscall(val) {
154-
this[kInfo].syscall = val;
155-
}
156-
157-
get path() {
158-
return this[kInfo].path !== undefined ?
159-
this[kInfo].path.toString() : undefined;
160-
}
119+
Object.defineProperty(this, 'errno', {
120+
get() {
121+
return context.errno;
122+
},
123+
set: (value) => {
124+
context.errno = value;
125+
},
126+
enumerable: true,
127+
configurable: true
128+
});
161129

162-
set path(val) {
163-
this[kInfo].path = val ?
164-
lazyBuffer().from(val.toString()) : undefined;
165-
}
130+
Object.defineProperty(this, 'syscall', {
131+
get() {
132+
return context.syscall;
133+
},
134+
set: (value) => {
135+
context.syscall = value;
136+
},
137+
enumerable: true,
138+
configurable: true
139+
});
166140

167-
get dest() {
168-
return this[kInfo].path !== undefined ?
169-
this[kInfo].dest.toString() : undefined;
170-
}
141+
if (context.path !== undefined) {
142+
Object.defineProperty(this, 'path', {
143+
get() {
144+
return context.path != null ?
145+
context.path.toString() : context.path;
146+
},
147+
set: (value) => {
148+
context.path = value ?
149+
lazyBuffer().from(value.toString()) : undefined;
150+
},
151+
enumerable: true,
152+
configurable: true
153+
});
154+
}
171155

172-
set dest(val) {
173-
this[kInfo].dest = val ?
174-
lazyBuffer().from(val.toString()) : undefined;
156+
if (context.dest !== undefined) {
157+
Object.defineProperty(this, 'dest', {
158+
get() {
159+
return context.dest != null ?
160+
context.dest.toString() : context.dest;
161+
},
162+
set: (value) => {
163+
context.dest = value ?
164+
lazyBuffer().from(value.toString()) : undefined;
165+
},
166+
enumerable: true,
167+
configurable: true
168+
});
169+
}
175170
}
176171

177172
toString() {
178173
return `${this.name} [${this.code}]: ${this.message}`;
179174
}
175+
176+
[Symbol.for('nodejs.util.inspect.custom')](recurseTimes, ctx) {
177+
return lazyInternalUtilInspect().inspect(this, {
178+
...ctx,
179+
getters: true,
180+
customInspect: false
181+
});
182+
}
180183
}
181184

182185
function makeSystemErrorWithCode(key) {
@@ -207,19 +210,7 @@ function makeNodeErrorWithCode(Base, key) {
207210
configurable: true
208211
});
209212
addCodeToName(this, super.name, key);
210-
}
211-
212-
get code() {
213-
return key;
214-
}
215-
216-
set code(value) {
217-
Object.defineProperty(this, 'code', {
218-
configurable: true,
219-
enumerable: true,
220-
value,
221-
writable: true
222-
});
213+
this.code = key;
223214
}
224215

225216
toString() {
@@ -380,7 +371,6 @@ function uvException(ctx) {
380371
err[prop] = ctx[prop];
381372
}
382373

383-
// TODO(BridgeAR): Show the `code` property as part of the stack.
384374
err.code = code;
385375
if (path) {
386376
err.path = path;

‎test/message/internal_assert.out‎

Lines 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+
}

‎test/message/internal_assert_fail.out‎

Lines 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+
}

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

Lines 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();

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

Lines 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);

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

Lines 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)