FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: return void in InitializeInspector() · nodejs/node@824dcfc · GitHub

/ node Public

Commit 824dcfc

Browse files
joyeecheung
authored and
RafaelGSS
committed
src: return void in InitializeInspector()
We have been ignoring inspector port binding errors during startup. Handling this error would be a breaking change and it's probably surprising to refuse to launch the Node.js instance simply because the inspector cannot listen to the port anyway. So just turn the return value of InitializeInspector() and remove the TODOs for handling the error. PR-URL: #44903 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent b2e6048 commit 824dcfc

6 files changed

Lines changed: 63 additions & 11 deletions

File tree

‎src/api/environment.cc‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,6 @@ Environment* CreateEnvironment(
370370
Environment* env = new Environment(
371371
isolate_data, context, args, exec_args, nullptr, flags, thread_id);
372372
#if HAVE_INSPECTOR
373-
// TODO(joyeecheung): handle the exit code returned by InitializeInspector().
374373
if (env->should_create_inspector()) {
375374
if (inspector_parent_handle) {
376375
env->InitializeInspector(

‎src/env.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,7 @@ class Environment : public MemoryRetainer {
617617
#if HAVE_INSPECTOR
618618
// If the environment is created for a worker, pass parent_handle and
619619
// the ownership if transferred into the Environment.
620-
ExitCode InitializeInspector(
620+
void InitializeInspector(
621621
std::unique_ptr<inspector::ParentInspectorHandle> parent_handle);
622622
#endif
623623

‎src/node.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ void SignalExit(int signo, siginfo_t* info, void* ucontext) {
166166
#endif // __POSIX__
167167

168168
#if HAVE_INSPECTOR
169-
ExitCode Environment::InitializeInspector(
169+
void Environment::InitializeInspector(
170170
std::unique_ptr<inspector::ParentInspectorHandle> parent_handle) {
171171
std::string inspector_path;
172172
bool is_main = !parent_handle;
@@ -187,7 +187,7 @@ ExitCode Environment::InitializeInspector(
187187
is_main);
188188
if (options_->debug_options().inspector_enabled &&
189189
!inspector_agent_->IsListening()) {
190-
return ExitCode::kInvalidCommandLineArgument2; // Signal internal error
190+
return;
191191
}
192192

193193
profiler::StartProfilers(this);
@@ -196,7 +196,7 @@ ExitCode Environment::InitializeInspector(
196196
inspector_agent_->PauseOnNextJavascriptStatement("Break at bootstrap");
197197
}
198198

199-
return ExitCode::kNoFailure;
199+
return;
200200
}
201201
#endif // HAVE_INSPECTOR
202202

‎src/node_main_instance.cc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,6 @@ NodeMainInstance::CreateMainEnvironment(ExitCode* exit_code) {
181181
SetIsolateErrorHandlers(isolate_, {});
182182
env->InitializeMainContext(context, &(snapshot_data_->env_info));
183183
#if HAVE_INSPECTOR
184-
// TODO(joyeecheung): handle the exit code returned by
185-
// InitializeInspector().
186184
env->InitializeInspector({});
187185
#endif
188186

@@ -201,8 +199,6 @@ NodeMainInstance::CreateMainEnvironment(ExitCode* exit_code) {
201199
EnvironmentFlags::kDefaultFlags,
202200
{}));
203201
#if HAVE_INSPECTOR
204-
// TODO(joyeecheung): handle the exit code returned by
205-
// InitializeInspector().
206202
env->InitializeInspector({});
207203
#endif
208204
if (env->principal_realm()->RunBootstrapping().IsEmpty()) {

‎src/node_snapshotable.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,8 +1173,6 @@ ExitCode SnapshotBuilder::Generate(SnapshotData* out,
11731173
// in the future).
11741174
if (snapshot_type == SnapshotMetadata::Type::kFullyCustomized) {
11751175
#if HAVE_INSPECTOR
1176-
// TODO(joyeecheung): handle the exit code returned by
1177-
// InitializeInspector().
11781176
env->InitializeInspector({});
11791177
#endif
11801178
if (LoadEnvironment(env, StartExecutionCallback{}).IsEmpty()) {

‎test/parallel/test-inspect-address-in-use.js‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
'use strict';
2+
const common = require('../common');
3+
common.skipIfInspectorDisabled();
4+
5+
const { spawnSync } = require('child_process');
6+
const { createServer } = require('http');
7+
const assert = require('assert');
8+
const fixtures = require('../common/fixtures');
9+
const entry = fixtures.path('empty.js');
10+
const { Worker } = require('worker_threads');
11+
12+
function testOnServerListen(fn) {
13+
const server = createServer((socket) => {
14+
socket.end('echo');
15+
});
16+
17+
server.on('listening', () => {
18+
fn(server);
19+
server.close();
20+
});
21+
server.listen(0, '127.0.0.1');
22+
}
23+
24+
function testChildProcess(getArgs, exitCode) {
25+
testOnServerListen((server) => {
26+
const { port } = server.address();
27+
const child = spawnSync(process.execPath, getArgs(port));
28+
const stderr = child.stderr.toString().trim();
29+
const stdout = child.stdout.toString().trim();
30+
console.log('[STDERR]');
31+
console.log(stderr);
32+
console.log('[STDOUT]');
33+
console.log(stdout);
34+
const match = stderr.match(
35+
/Starting inspector on 127\.0\.0\.1:(\d+) failed: address already in use/
36+
);
37+
assert.notStrictEqual(match, null);
38+
assert.strictEqual(match[1], port + '');
39+
assert.strictEqual(child.status, exitCode);
40+
});
41+
}
42+
43+
testChildProcess(
44+
(port) => [`--inspect=${port}`, '--build-snapshot', entry], 0);
45+
testChildProcess(
46+
(port) => [`--inspect=${port}`, entry], 0);
47+
48+
testOnServerListen((server) => {
49+
const { port } = server.address();
50+
const worker = new Worker(entry, {
51+
execArgv: [`--inspect=${port}`]
52+
});
53+
54+
worker.on('error', common.mustNotCall());
55+
56+
worker.on('exit', common.mustCall((code) => {
57+
assert.strictEqual(code, 0);
58+
}));
59+
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL