Support V8 Inspector Integration for Node.js

[novascreen]

Changes deprecated --debug to --inspect when using LaunchWithDebugging.

I have not implemented inspect-brk. Node 6 uses debug-brk so maybe this is enough for this feature and details like that might be better addressed by a separate feature which allows users to pass any parameter, as discussed in #796.

Node logs 4 separate messages for the debugger initially, so I wasn't sure what you would do to handle those. I've decided to catch any logs related to the debugger and log them as warnings.

One of the messages is a warning, but terminated the dotnet process, so I created a separate check for that (StartsWith("Warning") and log that as a warning as well.

Let me know if there is anything you'd like me to improve.

[dnfclas]

@novascreen,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

[novascreen]

I just noticed that a DeprecationWarning thrown from Node also terminates the dotnet process, i could add that to the check and fix that as part of the PR or create a new one in which I take care of both warnings. Or maybe this should be handled somewhere else?

[SteveSandersonMS]

Thanks - this is great too :) Merged.

I did make one small tweak to remove the IsWarning check because it would have been a breaking change (it would mean that lines starting Warning: no longer get passed to OnErrorDataReceived, which might have been overridden to do something with that data). Although we can make breaking changes if they are important, I think this change wasn't strictly required, so it's better to go ahead without it.

Everything to do with how you got --inspect running, and updated the docs, was all perfect - thanks so much!

[novascreen]

@SteveSandersonMS I see this was recently published. But i don't understand how i would catch that warning now so the dotnet process doesn't fail instantly when it comes across that warning?
Maybe should specifically add Warning: This is an experimental fe ature and could change at any time. to IsDebuggerMessage?

[SteveSandersonMS]

@novascreen Yes, I'm seeing that now too. It didn't happen when I originally tried this back in March, but it does now on more up-to-date versions of Node. It's unfortunate that we don't have a way of knowing exactly which warning messages relate to the debugger, given that future versions of Node might rephrase this in arbitrary ways.

So yes, we do need to capture that warning for this to work. I'd rather the code be more specific than just matching everything that starts with Warning:, because we don't want to suppress unrelated warnings.

Are you open to submitting a follow-up PR that catches the specific warning text we're seeing now?

[novascreen]

for sure, I'll do that