Skip to content
This repository has been archived by the owner on Feb 1, 2022. It is now read-only.

Internal error in node-inspect. Please report this bug. #60

Closed
LEQADA opened this issue Mar 21, 2018 · 5 comments
Closed

Internal error in node-inspect. Please report this bug. #60

LEQADA opened this issue Mar 21, 2018 · 5 comments

Comments

@LEQADA
Copy link

LEQADA commented Mar 21, 2018

Description

When the port is busy – node-inspect throws a timeout exception which is logged as an internal error.

  • Version: v9.7.1
  • Platform: Windows 10 x64
  • Subsystem: node-inspect
node debug .
(node:14108) [DEP0068] DeprecationWarning: `node debug` is deprecated. Please use `node inspect` instead.
There was an internal error in node-inspect. Please report this bug.
Timeout (2000) waiting for 127.0.0.1:9229 to be free
Error: Timeout (2000) waiting for 127.0.0.1:9229 to be free
    at Timeout.setTimeout [as _onTimeout] (node-inspect/lib/_inspect.js:54:14)
    at ontimeout (timers.js:466:11)
    at tryOnTimeout (timers.js:304:5)
    at Timer.listOnTimeout (timers.js:267:5)

Overview

Timeout message is generated here

setTimeout(() => {
didTimeOut = true;
reject(new Error(
`Timeout (${timeout}) waiting for ${host}:${port} to be free`));
}, timeout);

and then catched by a handler

function handleUnexpectedError(e) {
console.error('There was an internal error in node-inspect. ' +
'Please report this bug.');
console.error(e.message);
console.error(e.stack);
if (inspector.child) inspector.child.kill();
process.exit(1);
}

@jkrems
Copy link
Collaborator

jkrems commented Mar 21, 2018

Suggestion by @bnoordhuis:

Would logging a message followed by process.exit(1) in the timer callback work?

Suggestion by @jkrems:

Alternatively: making it a non-fatal error. "Failed to start app. Please check that the port isn't used by anything else. Use run to try again." etc.. Another solution would be to use a custom code and/or error class (StartupError) and exclude that from the internal error message. But that seems a bit more hacky.

@LEQADA
Copy link
Author

LEQADA commented Mar 21, 2018

@bnoordhuis I think it will work. But I'm not sure is it good to process.exit(1) in 2 places 🤔
@jkrems I'm not quite sure what you mean by making it non-fatal error. To not reject? Create another type of handler? Could you give me an example?

@jkrems
Copy link
Collaborator

jkrems commented Mar 21, 2018

By "non-fatal error" I meant starting the repl instead of exiting the node-inspect process. Then the user just needs to type restart to retry if they fixed the other process. That might also address cases where the app crashed, the user is trying to run it again (run<enter>), and they run into the same problem (another process is blocking the port).

@jkrems
Copy link
Collaborator

jkrems commented Mar 21, 2018

But I'd be more than happy to merge a simpler solution that just hides away the "internal error" message somehow. There's always time to improve the UX even further in the future.

@jkrems
Copy link
Collaborator

jkrems commented May 31, 2018

Potential fix is here: #62

@jkrems jkrems closed this as completed in d278b23 May 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants