Skip to content

Commit 64959b6

Browse files
addaleaxaddaleax
committed
report: remove internalBinding('config').hasReport
The `setup()` method is only called when the `--experimental-report` option is set. `getOptionValue()` returns `undefined` when the flag is not defined, so the extra check inside of `setup()` is redundant. PR-URL: #25610 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
1 parent 9d8a225 commit 64959b6

File tree

2 files changed

+132
-137
lines changed

2 files changed

+132
-137
lines changed

‎lib/internal/process/report.js‎

Lines changed: 132 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -11,153 +11,152 @@ exports.setup = function() {
1111
const REPORTFILENAME = 3;
1212
const REPORTPATH = 4;
1313
const REPORTVERBOSE = 5;
14-
if (internalBinding('config').hasReport) {
15-
// If report is enabled, extract the binding and
16-
// wrap the APIs with thin layers, with some error checks.
17-
// user options can come in from CLI / ENV / API.
18-
// CLI and ENV is intercepted in C++ and the API call here (JS).
19-
// So sync up with both sides as appropriate - initially from
20-
// C++ to JS and from JS to C++ whenever the API is called.
21-
// Some events are controlled purely from JS (signal | exception)
22-
// and some from C++ (fatalerror) so this sync-up is essential for
23-
// correct behavior and alignment with the supplied tunables.
24-
const nr = internalBinding('report');
2514

26-
// Keep it un-exposed; lest programs play with it
27-
// leaving us with a lot of unwanted sanity checks.
28-
let config = {
29-
events: [],
30-
signal: 'SIGUSR2',
31-
filename: '',
32-
path: '',
33-
verbose: false
34-
};
35-
const report = {
36-
setDiagnosticReportOptions(options) {
37-
emitExperimentalWarning('report');
38-
// Reuse the null and undefined checks. Save
39-
// space when dealing with large number of arguments.
40-
const list = parseOptions(options);
15+
// If report is enabled, extract the binding and
16+
// wrap the APIs with thin layers, with some error checks.
17+
// user options can come in from CLI / ENV / API.
18+
// CLI and ENV is intercepted in C++ and the API call here (JS).
19+
// So sync up with both sides as appropriate - initially from
20+
// C++ to JS and from JS to C++ whenever the API is called.
21+
// Some events are controlled purely from JS (signal | exception)
22+
// and some from C++ (fatalerror) so this sync-up is essential for
23+
// correct behavior and alignment with the supplied tunables.
24+
const nr = internalBinding('report');
4125

42-
// Flush the stale entries from report, as
43-
// we are refreshing it, items that the users did not
44-
// touch may be hanging around stale otherwise.
45-
config = {};
26+
// Keep it un-exposed; lest programs play with it
27+
// leaving us with a lot of unwanted sanity checks.
28+
let config = {
29+
events: [],
30+
signal: 'SIGUSR2',
31+
filename: '',
32+
path: '',
33+
verbose: false
34+
};
35+
const report = {
36+
setDiagnosticReportOptions(options) {
37+
emitExperimentalWarning('report');
38+
// Reuse the null and undefined checks. Save
39+
// space when dealing with large number of arguments.
40+
const list = parseOptions(options);
4641

47-
// The parseOption method returns an array that include
48-
// the indices at which valid params are present.
49-
list.forEach((i) => {
50-
switch (i) {
51-
case REPORTEVENTS:
52-
if (Array.isArray(options.events))
53-
config.events = options.events;
54-
else
55-
throw new ERR_INVALID_ARG_TYPE('events',
56-
'Array',
57-
options.events);
58-
break;
59-
case REPORTSIGNAL:
60-
if (typeof options.signal !== 'string') {
61-
throw new ERR_INVALID_ARG_TYPE('signal',
62-
'String',
63-
options.signal);
64-
}
65-
process.removeListener(config.signal, handleSignal);
66-
if (config.events.includes('signal'))
67-
process.on(options.signal, handleSignal);
68-
config.signal = options.signal;
69-
break;
70-
case REPORTFILENAME:
71-
if (typeof options.filename !== 'string') {
72-
throw new ERR_INVALID_ARG_TYPE('filename',
73-
'String',
74-
options.filename);
75-
}
76-
config.filename = options.filename;
77-
break;
78-
case REPORTPATH:
79-
if (typeof options.path !== 'string')
80-
throw new ERR_INVALID_ARG_TYPE('path', 'String', options.path);
81-
config.path = options.path;
82-
break;
83-
case REPORTVERBOSE:
84-
if (typeof options.verbose !== 'string' &&
85-
typeof options.verbose !== 'boolean') {
86-
throw new ERR_INVALID_ARG_TYPE('verbose',
87-
'Booelan | String' +
88-
' (true|false|yes|no)',
89-
options.verbose);
90-
}
91-
config.verbose = options.verbose;
92-
break;
93-
}
94-
});
95-
// Upload this new config to C++ land
96-
nr.syncConfig(config, true);
97-
},
42+
// Flush the stale entries from report, as
43+
// we are refreshing it, items that the users did not
44+
// touch may be hanging around stale otherwise.
45+
config = {};
46+
47+
// The parseOption method returns an array that include
48+
// the indices at which valid params are present.
49+
list.forEach((i) => {
50+
switch (i) {
51+
case REPORTEVENTS:
52+
if (Array.isArray(options.events))
53+
config.events = options.events;
54+
else
55+
throw new ERR_INVALID_ARG_TYPE('events',
56+
'Array',
57+
options.events);
58+
break;
59+
case REPORTSIGNAL:
60+
if (typeof options.signal !== 'string') {
61+
throw new ERR_INVALID_ARG_TYPE('signal',
62+
'String',
63+
options.signal);
64+
}
65+
process.removeListener(config.signal, handleSignal);
66+
if (config.events.includes('signal'))
67+
process.on(options.signal, handleSignal);
68+
config.signal = options.signal;
69+
break;
70+
case REPORTFILENAME:
71+
if (typeof options.filename !== 'string') {
72+
throw new ERR_INVALID_ARG_TYPE('filename',
73+
'String',
74+
options.filename);
75+
}
76+
config.filename = options.filename;
77+
break;
78+
case REPORTPATH:
79+
if (typeof options.path !== 'string')
80+
throw new ERR_INVALID_ARG_TYPE('path', 'String', options.path);
81+
config.path = options.path;
82+
break;
83+
case REPORTVERBOSE:
84+
if (typeof options.verbose !== 'string' &&
85+
typeof options.verbose !== 'boolean') {
86+
throw new ERR_INVALID_ARG_TYPE('verbose',
87+
'Booelan | String' +
88+
' (true|false|yes|no)',
89+
options.verbose);
90+
}
91+
config.verbose = options.verbose;
92+
break;
93+
}
94+
});
95+
// Upload this new config to C++ land
96+
nr.syncConfig(config, true);
97+
},
9898

9999

100-
triggerReport(file, err) {
101-
emitExperimentalWarning('report');
102-
if (err == null) {
103-
if (file == null) {
104-
return nr.triggerReport(new ERR_SYNTHETIC(
105-
'JavaScript Callstack').stack);
106-
}
107-
if (typeof file !== 'string')
108-
throw new ERR_INVALID_ARG_TYPE('file', 'String', file);
109-
return nr.triggerReport(file, new ERR_SYNTHETIC(
100+
triggerReport(file, err) {
101+
emitExperimentalWarning('report');
102+
if (err == null) {
103+
if (file == null) {
104+
return nr.triggerReport(new ERR_SYNTHETIC(
110105
'JavaScript Callstack').stack);
111106
}
112-
if (typeof err !== 'object')
113-
throw new ERR_INVALID_ARG_TYPE('err', 'Object', err);
114-
if (file == null)
115-
return nr.triggerReport(err.stack);
116107
if (typeof file !== 'string')
117108
throw new ERR_INVALID_ARG_TYPE('file', 'String', file);
118-
return nr.triggerReport(file, err.stack);
119-
},
120-
getReport(err) {
121-
emitExperimentalWarning('report');
122-
if (err == null) {
123-
return nr.getReport(new ERR_SYNTHETIC('JavaScript Callstack').stack);
124-
} else if (typeof err !== 'object') {
125-
throw new ERR_INVALID_ARG_TYPE('err', 'Objct', err);
126-
} else {
127-
return nr.getReport(err.stack);
128-
}
109+
return nr.triggerReport(file, new ERR_SYNTHETIC(
110+
'JavaScript Callstack').stack);
129111
}
130-
};
112+
if (typeof err !== 'object')
113+
throw new ERR_INVALID_ARG_TYPE('err', 'Object', err);
114+
if (file == null)
115+
return nr.triggerReport(err.stack);
116+
if (typeof file !== 'string')
117+
throw new ERR_INVALID_ARG_TYPE('file', 'String', file);
118+
return nr.triggerReport(file, err.stack);
119+
},
120+
getReport(err) {
121+
emitExperimentalWarning('report');
122+
if (err == null) {
123+
return nr.getReport(new ERR_SYNTHETIC('JavaScript Callstack').stack);
124+
} else if (typeof err !== 'object') {
125+
throw new ERR_INVALID_ARG_TYPE('err', 'Objct', err);
126+
} else {
127+
return nr.getReport(err.stack);
128+
}
129+
}
130+
};
131131

132-
// Download the CLI / ENV config into JS land.
133-
nr.syncConfig(config, false);
132+
// Download the CLI / ENV config into JS land.
133+
nr.syncConfig(config, false);
134134

135-
function handleSignal(signo) {
136-
if (typeof signo !== 'string')
137-
signo = config.signal;
138-
nr.onUserSignal(signo);
139-
}
135+
function handleSignal(signo) {
136+
if (typeof signo !== 'string')
137+
signo = config.signal;
138+
nr.onUserSignal(signo);
139+
}
140140

141-
if (config.events.includes('signal')) {
142-
process.on(config.signal, handleSignal);
143-
}
141+
if (config.events.includes('signal')) {
142+
process.on(config.signal, handleSignal);
143+
}
144144

145-
function parseOptions(obj) {
146-
const list = [];
147-
if (obj == null)
148-
return list;
149-
if (obj.events != null)
150-
list.push(REPORTEVENTS);
151-
if (obj.signal != null)
152-
list.push(REPORTSIGNAL);
153-
if (obj.filename != null)
154-
list.push(REPORTFILENAME);
155-
if (obj.path != null)
156-
list.push(REPORTPATH);
157-
if (obj.verbose != null)
158-
list.push(REPORTVERBOSE);
145+
function parseOptions(obj) {
146+
const list = [];
147+
if (obj == null)
159148
return list;
160-
}
161-
process.report = report;
149+
if (obj.events != null)
150+
list.push(REPORTEVENTS);
151+
if (obj.signal != null)
152+
list.push(REPORTSIGNAL);
153+
if (obj.filename != null)
154+
list.push(REPORTFILENAME);
155+
if (obj.path != null)
156+
list.push(REPORTPATH);
157+
if (obj.verbose != null)
158+
list.push(REPORTVERBOSE);
159+
return list;
162160
}
161+
process.report = report;
163162
};

‎src/node_config.cc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,6 @@ static void Initialize(Local<Object> target,
7373

7474
#endif // NODE_HAVE_I18N_SUPPORT
7575

76-
#if defined(NODE_REPORT)
77-
READONLY_TRUE_PROPERTY(target, "hasReport");
78-
#endif // NODE_REPORT
79-
8076
if (env->options()->preserve_symlinks)
8177
READONLY_TRUE_PROPERTY(target, "preserveSymlinks");
8278
if (env->options()->preserve_symlinks_main)

0 commit comments

Comments
 (0)