doc: Deprecate old debug protocol #10320

jkrems · 2016-12-17T18:57:29Z

Checklist

documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

doc

Description of change

As discussed in #10276, we'd want to add a soft deprecation of the --debug flag to the docs.

This just adds the deprecation to the "advanced usage" section, not to the overall page. I think we can actually keep supporting node debug <filename> since it doesn't leak the protocol used (as pid and url do, both of which could have been started with a different version of node).

Trott · 2016-12-18T04:49:30Z

doc/api/debugger.md

@@ -179,12 +183,13 @@ process or via URI reference to the listening debugger:
 * `node debug <URI>` - Connects to the process via the URI such as
 localhost:5858

-## V8 Inspector Integration for Node.js
+### V8 Inspector Integration for Node.js

 **NOTE: This is an experimental feature.**


I wonder if we should remove the "experimental feature" note, especially if we're pointing people to it as an alternative to the deprecated debugger. That doesn't have to happen in this PR (although it could if there's consensus that it should happen).

Yeah, I was thinking about what the "least worst" option would be. It seemed also bad to remove the doc warning while the runtime still writes one ("docs say go ahead but then the runtime punishes the user with scary warnings").

P.S.: The solution could be to remove the warning in this PR, but there's #8978 already touching those parts.

Trott · 2016-12-18T04:51:35Z

@nodejs/ctc This is something we should probably decide on sooner rather than later. We may need to create an exception to our deprecation policy for the debugger situation. We may not have much of a choice (unless we want to not upgrade V8 for 18 months).

addaleax · 2016-12-19T04:16:08Z

doc/api/debugger.md

@@ -167,6 +167,10 @@ breakpoint)

 ## Advanced Usage

+### TCP-based protocol
+
+> Stability: 0 - Deprecated: Use [V8 Inspector Integration][] instead. The debug protocol used by the `--debug` flag was removed from V8.


nit: Can you wrap this at 80 characters? You should be able to use multiple >s to continue the blockquote, the documentation generator should be able to handle that

I had it on multiple lines at first but line breaks inside of > were preserved which looked awkward in the rendered version.

@jkrems It's fixed in #11074

jkrems · 2016-12-22T02:20:15Z

Rebased, only conflict was the changed link at the very bottom.

mhdawson

LGTM

Trott · 2017-01-04T03:31:07Z

Are there semver implications for a docs-only deprecation? I'd like to land this sooner rather than later...

jasnell · 2017-01-04T03:42:29Z

All deprecations are semver major, including docs only

…

On Tue, Jan 3, 2017 at 7:31 PM Rich Trott ***@***.***> wrote: Are there semver implications for a docs-only deprecation? I'd like to land this sooner rather than later... — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#10320 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAa2eZc3Jctp9F3uWMdU-bW95ZsLsqdUks5rOxJ_gaJpZM4LP9Kh> .

Trott · 2017-01-04T04:14:52Z

All deprecations are semver major, including docs only

Can we and should we make an exception for this one? If I'm not mistaken, we expect the debugger to be not-working come Node.js 8, so a docs-only deprecation now would seem to be the kind thing to do for our users as opposed to having it go away with no warning.

Fishrock123 · 2017-01-04T04:22:53Z

All deprecations are semver major, including docs only

heh, it's not like we've approved deprecations in-major before when we had no other choice (smalloc)

jasnell · 2017-01-04T05:07:45Z

I'd rather not start getting into the habit of making exceptions for deprecations. If we want to treat docs only deprecations as anything less than semver-major, then we should establish that as policy and do all docs only deprecations as less than semver-major.

targos · 2017-01-04T15:59:55Z

Correct me if I'm wrong, but I think that for this case, we must make an exception to our policy. We have to choose between:

a semver-minor docs only deprecation in 7.x, followed by the removal of the feature in 8.0
direct removal in 8.0 without deprecation

Trott · 2017-01-04T17:26:59Z

Consensus at CTC meeting today was that this can move forward.

(This and another issue highlighted how we need to more concretely document our semver policy and @jasnell is restarting an earlier effort to do that.)

Removing ctc-agenda label. Adding labels for hopefully landing this on v6 as well so that people who only use LTS versions have a chance of noticing the doc note.

Due to changes in V8, the debug protocol will no longer work in Node.js 8.0.0. Note this in the documentation. PR-URL: nodejs#10320 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

Trott · 2017-01-04T17:30:40Z

Landed in 59a61a2.

@jkrems Do we need to runtime deprecate the debug protocol too? Or are we thinking that there's a chance it can be an alias for inspect without too much fallout? Or another option? /cc @Fishrock123

jkrems · 2017-01-04T17:45:43Z

@Trott The runtime deprecation was already kicked off by @joshgav here: #10276. But yes, warning at runtime (at least in 7.x, maybe/hopefully even in 6.x) would be super useful I think.

Trott · 2017-01-10T07:32:08Z

Do we need to deprecate vm.runInDebugContext() too? Or is that likely to continue working?

targos · 2017-01-10T07:42:39Z

I'm not sure. It's using the v8::Debug C++ API.
/cc @hashseed

hashseed · 2017-01-10T07:47:45Z

vm.runInDebugContext uses the debug context. That is mostly unrelated to the legacy JSON protocol (which will be removed by V8 5.8). We will deprecate debug cont wi the API to access the debug context will be marked accordingly in include/v8.h, but not in the short term.

evanlucas · 2017-01-31T16:24:28Z

This doesn't land cleanly on v7.x. Someone want to backport?

jkrems · 2017-02-04T17:58:31Z

@evanlucas Backport = cherry-pick commit against 7.x, resolve conflicts, PR against 7.x branch? If so I could give it a shot.

evanlucas · 2017-02-04T18:10:46Z

Yes, but Pr against v7.x-staging instead of v7.x

Thanks!

jkrems · 2017-02-04T22:55:05Z

Resolved here: #11172

Due to changes in V8, the debug protocol will no longer work in Node.js 8.0.0. Note this in the documentation. PR-URL: nodejs#10320 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

jkrems · 2017-03-20T19:08:02Z

The backport PR has been updated again. I really think we should get this into the published docs rather sooner than later, given how close we are to adding runtime deprecations (or even dropping support).

Due to changes in V8, the debug protocol will no longer work in Node.js 8.0.0. Note this in the documentation. PR-URL: nodejs#10320 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

nodejs-github-bot added debugger doc Issues and PRs related to the documentations. labels Dec 17, 2016

This was referenced Dec 17, 2016

src, lib: deprecate old debug-agent and CLI debugger #10276

Closed

Bringing node-inspect into the fold nodejs/diagnostics#67

Closed

Trott reviewed Dec 18, 2016

View reviewed changes

Trott added dont-land-on-v4.x and removed lts-watch-v6.x labels Dec 18, 2016

addaleax approved these changes Dec 19, 2016

View reviewed changes

jasnell approved these changes Dec 22, 2016

View reviewed changes

doc: Deprecate old debug protocol

65c3034

jkrems force-pushed the jk-v8-protocol-deprecation branch from 45e479d to 65c3034 Compare December 22, 2016 02:19

nodejs-github-bot added the dont-land-on-v7.x label Dec 22, 2016

Trott removed the dont-land-on-v7.x label Dec 22, 2016

mhdawson approved these changes Jan 3, 2017

View reviewed changes

Trott added the ctc-agenda label Jan 4, 2017

Trott mentioned this pull request Jan 4, 2017

Node.js Foundation Core Technical Committee (CTC) Meeting 2017-01-04 nodejs/CTC#54

Closed

Trott added lts-watch-v6.x and removed ctc-agenda labels Jan 4, 2017

Trott added dont-land-on-v4.x notable-change PRs with changes that should be highlighted in changelogs. and removed dont-land-on-v6.x labels Jan 4, 2017

Trott closed this Jan 4, 2017

jkrems deleted the jk-v8-protocol-deprecation branch January 4, 2017 17:46

italoacasas mentioned this pull request Jan 19, 2017

v7.x-staging update proposal #10886

Closed

joshgav mentioned this pull request Jan 23, 2017

lib: deprecate node --debug at runtime #10970

Closed

3 tasks

italoacasas mentioned this pull request Jan 25, 2017

Proposal to update v7.x-staging #10989

Closed

evanlucas added the dont-land-on-v7.x label Jan 31, 2017

jkrems mentioned this pull request Feb 4, 2017

backport: Deprecate old debug protocol in docs #11172

Closed

2 tasks

jkrems mentioned this pull request Feb 16, 2017

Switch the CLI debugger to V8 inspector #11421

Closed

14 tasks

italoacasas mentioned this pull request Mar 21, 2017

v7.7.4 Release Proposal #11941

Merged

MylesBorins added the dont-land-on-v6.x label Apr 18, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: Deprecate old debug protocol #10320

doc: Deprecate old debug protocol #10320

jkrems commented Dec 17, 2016 •

edited

Loading

Trott Dec 18, 2016

jkrems Dec 18, 2016 •

edited

Loading

Trott commented Dec 18, 2016

addaleax Dec 19, 2016

jkrems Dec 19, 2016

thefourtheye Feb 4, 2017

jkrems commented Dec 22, 2016

mhdawson left a comment

Trott commented Jan 4, 2017

jasnell commented Jan 4, 2017 via email

Trott commented Jan 4, 2017

Fishrock123 commented Jan 4, 2017

jasnell commented Jan 4, 2017

targos commented Jan 4, 2017

Trott commented Jan 4, 2017

Trott commented Jan 4, 2017

jkrems commented Jan 4, 2017

Trott commented Jan 10, 2017

targos commented Jan 10, 2017

hashseed commented Jan 10, 2017 •

edited

Loading

evanlucas commented Jan 31, 2017

jkrems commented Feb 4, 2017

evanlucas commented Feb 4, 2017

jkrems commented Feb 4, 2017

jkrems commented Mar 20, 2017

doc: Deprecate old debug protocol #10320

doc: Deprecate old debug protocol #10320

Conversation

jkrems commented Dec 17, 2016 • edited Loading

Checklist

Affected core subsystem(s)

Description of change

Trott Dec 18, 2016

Choose a reason for hiding this comment

jkrems Dec 18, 2016 • edited Loading

Choose a reason for hiding this comment

Trott commented Dec 18, 2016

addaleax Dec 19, 2016

Choose a reason for hiding this comment

jkrems Dec 19, 2016

Choose a reason for hiding this comment

thefourtheye Feb 4, 2017

Choose a reason for hiding this comment

jkrems commented Dec 22, 2016

mhdawson left a comment

Choose a reason for hiding this comment

Trott commented Jan 4, 2017

jasnell commented Jan 4, 2017 via email

Trott commented Jan 4, 2017

Fishrock123 commented Jan 4, 2017

jasnell commented Jan 4, 2017

targos commented Jan 4, 2017

Trott commented Jan 4, 2017

Trott commented Jan 4, 2017

jkrems commented Jan 4, 2017

Trott commented Jan 10, 2017

targos commented Jan 10, 2017

hashseed commented Jan 10, 2017 • edited Loading

evanlucas commented Jan 31, 2017

jkrems commented Feb 4, 2017

evanlucas commented Feb 4, 2017

jkrems commented Feb 4, 2017

jkrems commented Mar 20, 2017

jkrems commented Dec 17, 2016 •

edited

Loading

jkrems Dec 18, 2016 •

edited

Loading

hashseed commented Jan 10, 2017 •

edited

Loading