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.

Sign up for GitHub

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

Jump to bottom

inspector: drop 'chrome-' from inspector url #33758

Merged
merged 2 commits into from
Jun 9, 2020
Merged

inspector: drop 'chrome-' from inspector url #33758

merged 2 commits into from
Jun 9, 2020

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Jun 5, 2020

It has been reported that the V8 inspector extension can no longer connect to DevTools (despite no code changes to the extension since Feb. 2017). Upon investigation, Chrome dropped support for the 'chrome-devtools:' scheme recently. I've confirmed that dropping 'chrome-' from the URL here allows the debugger to function properly with modern Chrome.

Refs: https://chromium.googlesource.com/chromium/src/+/6700d12448f76712c62a6d2372a95b97a26d4779

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol labels Jun 5, 2020
@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 5, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 5, 2020 •
edited by cjihrig
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/31736/ 💛

mscdex
mscdex reviewed Jun 5, 2020
View reviewed changes
doc/api/debugger.md
Debugger listening on 127.0.0.1:9229.
To start debugging, open the following URL in Chrome:
chrome-devtools://devtools/bundled/js_app.html?experiments=true&v8only=true&ws=127.0.0.1:9229/dc9010dd-f8b8-4ac5-a510-c1a114ec7d29
Debugger listening on ws://127.0.0.1:9229/dc9010dd-f8b8-4ac5-a510-c1a114ec7d29
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this URL no longer match the output format (e.g. missing /bundled/, etc.) from inspector_socket_server.cc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point, what is actually printed to the console changed. This is what is currently displayed.

@cjihrig
doc: update V8 inspector example
fd7e408 
This commit updates the V8 inspector example to reflect what
is currently printed to the console.

PR-URL: nodejs#33758
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
@cjihrig
inspector: drop 'chrome-' from inspector url
921f755 
It has been reported that the
https://github.com/cjihrig/node-v8-inspector V8 inspector
extension can no longer connect DevTools (despite no code changes
to the extension since Feb. 2017). Upon investigation, Chrome
dropped support for the chrome-devtools: scheme recently. I've
confirmed that dropping 'chrome-' from the URL here allows the
debugger to function properly with modern Chrome.

Refs: https://chromium.googlesource.com/chromium/src/+/6700d12448f76712c62a6d2372a95b97a26d4779
PR-URL: nodejs#33758
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
@cjihrig cjihrig merged commit 921f755 into nodejs:master Jun 9, 2020
@cjihrig cjihrig deleted the inspector branch June 9, 2020 01:33
@cjihrig
Copy link
Contributor Author

cjihrig commented Jun 9, 2020

Landed in 862d005...921f755. Thanks for the reviews!

codebytere pushed a commit that referenced this pull request Jun 18, 2020
@cjihrig @codebytere
doc: update V8 inspector example
b10d203 
This commit updates the V8 inspector example to reflect what
is currently printed to the console.

PR-URL: #33758
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
codebytere pushed a commit that referenced this pull request Jun 18, 2020
@cjihrig @codebytere
inspector: drop 'chrome-' from inspector url
e58f14f 
It has been reported that the
https://github.com/cjihrig/node-v8-inspector V8 inspector
extension can no longer connect DevTools (despite no code changes
to the extension since Feb. 2017). Upon investigation, Chrome
dropped support for the chrome-devtools: scheme recently. I've
confirmed that dropping 'chrome-' from the URL here allows the
debugger to function properly with modern Chrome.

Refs: https://chromium.googlesource.com/chromium/src/+/6700d12448f76712c62a6d2372a95b97a26d4779
PR-URL: #33758
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
@codebytere codebytere mentioned this pull request Jun 28, 2020
Merged
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@cjihrig @codebytere
doc: update V8 inspector example
e1b0651 
This commit updates the V8 inspector example to reflect what
is currently printed to the console.

PR-URL: #33758
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@cjihrig @codebytere
inspector: drop 'chrome-' from inspector url
ab668ab 
It has been reported that the
https://github.com/cjihrig/node-v8-inspector V8 inspector
extension can no longer connect DevTools (despite no code changes
to the extension since Feb. 2017). Upon investigation, Chrome
dropped support for the chrome-devtools: scheme recently. I've
confirmed that dropping 'chrome-' from the URL here allows the
debugger to function properly with modern Chrome.

Refs: https://chromium.googlesource.com/chromium/src/+/6700d12448f76712c62a6d2372a95b97a26d4779
PR-URL: #33758
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 10, 2020
@cjihrig @codebytere
doc: update V8 inspector example
046dee6 
This commit updates the V8 inspector example to reflect what
is currently printed to the console.

PR-URL: #33758
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 10, 2020
@cjihrig @codebytere
inspector: drop 'chrome-' from inspector url
103a9af 
It has been reported that the
https://github.com/cjihrig/node-v8-inspector V8 inspector
extension can no longer connect DevTools (despite no code changes
to the extension since Feb. 2017). Upon investigation, Chrome
dropped support for the chrome-devtools: scheme recently. I've
confirmed that dropping 'chrome-' from the URL here allows the
debugger to function properly with modern Chrome.

Refs: https://chromium.googlesource.com/chromium/src/+/6700d12448f76712c62a6d2372a95b97a26d4779
PR-URL: #33758
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
@codebytere codebytere mentioned this pull request Jul 13, 2020
Merged
codebytere pushed a commit that referenced this pull request Jul 14, 2020
@cjihrig @codebytere
doc: update V8 inspector example
69c9233 
This commit updates the V8 inspector example to reflect what
is currently printed to the console.

PR-URL: #33758
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 14, 2020
@cjihrig @codebytere
inspector: drop 'chrome-' from inspector url
26d6b2a 
It has been reported that the
https://github.com/cjihrig/node-v8-inspector V8 inspector
extension can no longer connect DevTools (despite no code changes
to the extension since Feb. 2017). Upon investigation, Chrome
dropped support for the chrome-devtools: scheme recently. I've
confirmed that dropping 'chrome-' from the URL here allows the
debugger to function properly with modern Chrome.

Refs: https://chromium.googlesource.com/chromium/src/+/6700d12448f76712c62a6d2372a95b97a26d4779
PR-URL: #33758
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mscdex mscdex mscdex left review comments

@trivikr trivikr trivikr approved these changes

@devnexen devnexen devnexen approved these changes

@mmarchini mmarchini mmarchini approved these changes

@addaleax addaleax addaleax approved these changes

@eugeneo eugeneo eugeneo approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@cjihrig @nodejs-github-bot @mscdex @eugeneo @addaleax @mmarchini @devnexen @trivikr