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: introduce the --inspect-wait flag #52734

Merged
merged 3 commits into from May 11, 2024
Merged

inspector: introduce the --inspect-wait flag #52734

merged 3 commits into from May 11, 2024
+90 −10

Conversation

cola119
Copy link
Member

@cola119 cola119 commented Apr 28, 2024 •
edited

This PR introduces the --inspect-wait flag, which allows debugger to wait for attachement. This flag is useful when you want to debug the code from the beginning. Unlike --inspect-brk, which breaks on the first line, this flag waits for debugger to be connected and then runs the code as soon as a session is established.

It's designed to simplify debugging by removing the need for manual intervention and providing a smoother debugging experience. With support for breaking on the first line and automatic process resumption, it streamlines the debugging process.

Other runtime works include:

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 28, 2024
@cola119 cola119 force-pushed the introduce-inspect-wait branch 2 times, most recently from 23e582e to 6422bd5 Compare April 28, 2024 14:01
@cola119 cola119 added the inspector Issues and PRs related to the V8 inspector protocol label Apr 28, 2024
@benjamingr
Copy link
Member

benjamingr commented Apr 28, 2024 •
edited

I'm not sure I understand the use case? Shouldn't this be solved from the consumer side that can call Debugger.resume (or some such) after attaching with --inspect-brk?

@cola119
Copy link
Member Author

cola119 commented Apr 29, 2024

Shouldn't this be solved from the consumer side that can call Debugger.resume (or some such) after attaching with --inspect-brk?

Yes, the --inspect-wait flag acts like a combination of --inspect-brk and the client's Debugger.resume call. It's designed to simplify debugging by removing the need for manual intervention and providing a smoother debugging experience. With support for breaking on the first line and automatic process resumption, I believe it streamlines the debugging process.

@cola119
Copy link
Member Author

cola119 commented Apr 30, 2024

cc @nodejs/inspector

@daeyeon
Copy link
Member

daeyeon commented May 1, 2024

Please update https://github.com/nodejs/node/blob/main/doc/node.1 also.

@cola119 cola119 added the request-ci Add this label to start a Jenkins CI on a PR. label May 1, 2024
@cola119
Copy link
Member Author

cola119 commented May 1, 2024

@daeyeon Done

@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels May 1, 2024
@github-actions GitHub Actions

This comment was marked as resolved.

Sign in to view
@daeyeon daeyeon added request-ci Add this label to start a Jenkins CI on a PR. semver-minor PRs that contain new features and should be released in the next minor version. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. semver-minor PRs that contain new features and should be released in the next minor version. labels May 3, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 3, 2024
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

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

jasnell
jasnell reviewed May 4, 2024
View reviewed changes
doc/api/cli.md
-->

Activate inspector on `host:port` and wait for debugger to be attached.
Default `host:port` is `127.0.0.1:9229`.
Copy link
Member

Choose a reason for hiding this comment

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

The docs should likely include additional detail explaining the difference between this and --inspect-brk. Just looking at this I would have no idea as an unfamiliar user which to choose and why.

jasnell
jasnell reviewed May 4, 2024
View reviewed changes
doc/api/debugger.md Outdated
Using the `--inspect` flag will execute the code immediately before debugger
is connected. This could be problematic if you intend to debug the code from the beginning.
In such cases, you can use the `--inspect-wait` flag instead, which waits for debugger to be attached,
or the `--inspect-brk` flag to break on the first line of the code.
Copy link
Member

Choose a reason for hiding this comment

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

This is good but could be expanded a bit to explain more of the difference. Why would I use one verses the other?

@cola119
Copy link
Member Author

cola119 commented May 5, 2024 •
edited

@jasnell @daeyeon I've updated the Node.js debugger documentation to clarify the difference between the three flags. Please review it again and let me know if you have any better ideas. ba10f45

@cola119 cola119 added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. request-ci Add this label to start a Jenkins CI on a PR. labels May 5, 2024
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels May 6, 2024
@github-actions GitHub Actions

This comment was marked as resolved.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

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

daeyeon
daeyeon approved these changes May 10, 2024
View reviewed changes
Copy link
Member

@daeyeon daeyeon left a comment

Choose a reason for hiding this comment

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

It would be good to get other folks reviewing the documentation if possible. As for the code, this LGTM.

@daeyeon daeyeon added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels May 10, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 10, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label May 11, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 11, 2024
@nodejs-github-bot nodejs-github-bot merged commit c0ae3b2 into nodejs:main May 11, 2024
59 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in c0ae3b2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@JungMinu JungMinu JungMinu approved these changes

@daeyeon daeyeon daeyeon 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++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. inspector Issues and PRs related to the V8 inspector protocol lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@cola119 @benjamingr @daeyeon @nodejs-github-bot @jasnell @JungMinu @aduh95