Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
avaer opened this issue Mar 13, 2018 · 9 comments
Closed

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

avaer opened this issue Mar 13, 2018 · 9 comments

Comments

@avaer
Copy link

avaer commented Mar 13, 2018

Just running node debug . tells me to report this bug. There doesn't need to be any actual module to run.

I think it's harmless and just complaining about the used port. But node explicitly told me to report this bug, so I'm following the instructions given 👍.

  • 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)
@Trott
Copy link
Member

Trott commented Mar 13, 2018

/ping @jkrems

@bnoordhuis
Copy link
Member

The 'internal error' message looks like a red herring to me, the real error is fairly benign: port 9229 is taken, probably by another node process.

@jkrems
Copy link
Contributor

jkrems commented Mar 13, 2018

Thanks for the report! Yeah, this seems silly (agreed with @bnoordhuis that it just seems to be the blocked port, not an actual bug) and we should fix the output for this case.

@LEQADA
Copy link

LEQADA commented Mar 17, 2018

I was thinking maybe we can add error.code = 'ECONNRESET'; to this error and in the handling function do something like this

    if(e.code !== 'ECONNRESET')
      console.error('There was an internal error in node-inspect. ' +
                    'Please report this bug.');

@jkrems @bnoordhuis wdyt ?

@LEQADA
Copy link

LEQADA commented Mar 20, 2018

Ping @jkrems @bnoordhuis. I can create a PR if you agree.

@bnoordhuis
Copy link
Member

@LEQADA Hm, that feels a little too much like a hack to me. Would logging a message followed by process.exit(1) in the timer callback work?

Note that a PR should go to nodejs/node-inspect. Once merged, it will make its way here on the next update.

@jkrems
Copy link
Contributor

jkrems commented Mar 20, 2018

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

LEQADA commented Mar 21, 2018

I duplicated this issue to the node-inspect repo. Should we move the discussion there?
nodejs/node-inspect#60

@jkrems
Copy link
Contributor

jkrems commented Mar 21, 2018

Moving discussion to node-inspect since that's where the issue needs to be resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants