inspector: add NodeRuntime.waitingForDebugger event #51560

mmarchini · 2024-01-24T19:53:50Z

NodeRuntime.waitingForDebugger is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when inspector.waitForDebugger() is called). This allows inspecting processes to know when the inspected process is waiting for a Runtime.runIfWaitingForDebugger message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following:

Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it
Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct
Simplify test-inspector-multisession-ws

There might be value in adding NodeWorker.waitingForDebugger in a future patch, but as of right now, no Node.js core inspector tests using worker threads are failing due to race conditions.

Fixes: #34730

nodejs-github-bot · 2024-01-24T19:53:55Z

Review requested:

@nodejs/test_runner

MoLow · 2024-01-25T06:57:04Z

linter seems to fail

koko1928

This appears to be failing in Linters.

`NodeRuntime.waitingForDebugger` is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when `inspector.waitForDebugger()` is called). This allows inspecting processes to know when the inspected process is waiting for a `Runtime.runIfWaitingForDebugger` message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following: * Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it * Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct * Simplify test-inspector-multisession-ws There might be value in adding `NodeWorker.waitingForDebugger` in a future patch, but as of right now, no Node.js core inspector tests using worker threads are not failing due to race conditions. Fixes: nodejs#34730

nodejs-github-bot · 2024-01-26T11:59:45Z

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

nodejs-github-bot · 2024-02-03T17:54:51Z

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

lpinca

RSLGTM

joyeecheung · 2024-02-23T17:16:47Z

Stress test for flaked tests on RHEL boxes: https://ci.nodejs.org/view/Stress/job/node-stress-single-test/475/ (reference nodejs/reliability#787)

nodejs-github-bot · 2024-02-23T17:21:24Z

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

nodejs-github-bot · 2024-02-23T21:03:37Z

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

nodejs-github-bot · 2024-02-23T22:46:30Z

Landed in 0161ad0

`NodeRuntime.waitingForDebugger` is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when `inspector.waitForDebugger()` is called). This allows inspecting processes to know when the inspected process is waiting for a `Runtime.runIfWaitingForDebugger` message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following: * Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it * Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct * Simplify test-inspector-multisession-ws There might be value in adding `NodeWorker.waitingForDebugger` in a future patch, but as of right now, no Node.js core inspector tests using worker threads are not failing due to race conditions. Fixes: #34730 PR-URL: #51560 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

`NodeRuntime.waitingForDebugger` is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when `inspector.waitForDebugger()` is called). This allows inspecting processes to know when the inspected process is waiting for a `Runtime.runIfWaitingForDebugger` message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following: * Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it * Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct * Simplify test-inspector-multisession-ws There might be value in adding `NodeWorker.waitingForDebugger` in a future patch, but as of right now, no Node.js core inspector tests using worker threads are not failing due to race conditions. Fixes: nodejs#34730 PR-URL: nodejs#51560 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

Refs: nodejs#51560

Refs: #51560 PR-URL: #54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

Use the `NodeRuntime.waitingForDebugger` event. Refs: nodejs#51560

Refs: #51560 PR-URL: #54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

Use the `NodeRuntime.waitingForDebugger` event. Refs: #51560 PR-URL: #55058 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Refs: #51560 PR-URL: #54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

Use the `NodeRuntime.waitingForDebugger` event. Refs: #51560 PR-URL: #55058 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Refs: nodejs#51560 PR-URL: nodejs#54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

Use the `NodeRuntime.waitingForDebugger` event. Refs: nodejs#51560 PR-URL: nodejs#55058 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Refs: nodejs#51560 PR-URL: nodejs#54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

Use the `NodeRuntime.waitingForDebugger` event. Refs: nodejs#51560 PR-URL: nodejs#55058 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Refs: nodejs#54827 Refs: nodejs#51560

Refs: #54827 Refs: #51560 PR-URL: #56050 Reviewed-By: Antoine du Hamel <[email protected]>

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 Jan 24, 2024

benjamingr approved these changes Jan 24, 2024

View reviewed changes

koko1928 reviewed Jan 25, 2024

View reviewed changes

mmarchini force-pushed the inspector/waitingForDebuggerEvent branch from 7c3a8e3 to ac5867d Compare January 25, 2024 19:53

mmarchini force-pushed the inspector/waitingForDebuggerEvent branch from ac5867d to 9ec34cb Compare January 25, 2024 19:59

richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 26, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 26, 2024

fhinkel approved these changes Jan 28, 2024

View reviewed changes

lpinca reviewed Feb 6, 2024

View reviewed changes

joyeecheung approved these changes Feb 23, 2024

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024

This comment was marked as abuse.

Sign in to view

joyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2024

nodejs-github-bot merged commit 0161ad0 into nodejs:main Feb 23, 2024

lpinca added a commit to lpinca/node that referenced this pull request Sep 7, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness

20c5cc9

Refs: nodejs#51560

This was referenced Sep 7, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness #54827

Merged

parallel.test-inspector-debug-brk-flag is flaky #49900

Closed

nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness

e48df8d

Refs: #51560 PR-URL: #54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

aduh95 pushed a commit that referenced this pull request Sep 12, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness

3638e51

Refs: #51560 PR-URL: #54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

aduh95 pushed a commit that referenced this pull request Sep 13, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness

03476dd

Refs: #51560 PR-URL: #54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

aduh95 pushed a commit that referenced this pull request Sep 13, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness

194cb83

Refs: #51560 PR-URL: #54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

lpinca mentioned this pull request Sep 17, 2024

test/parallel/test-runner-inspect.mjs #47418

Closed

lpinca added a commit to lpinca/node that referenced this pull request Sep 22, 2024

test: deflake test-inspector-strip-types

d1f4a31

Use the `NodeRuntime.waitingForDebugger` event. Refs: nodejs#51560

lpinca mentioned this pull request Sep 22, 2024

test: deflake test-inspector-strip-types #55058

Merged

targos pushed a commit that referenced this pull request Sep 22, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness

6b74b66

Refs: #51560 PR-URL: #54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

lpinca mentioned this pull request Sep 23, 2024

Flaky test-inspector-debug-end #49921

Closed

targos pushed a commit that referenced this pull request Sep 26, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness

20d0093

Refs: #51560 PR-URL: #54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

targos pushed a commit that referenced this pull request Oct 2, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness

94f242c

Refs: #51560 PR-URL: #54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

targos pushed a commit that referenced this pull request Oct 2, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness

60f5f54

Refs: #51560 PR-URL: #54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness

9d23914

Refs: nodejs#51560 PR-URL: nodejs#54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

tpoisseau pushed a commit to tpoisseau/node that referenced this pull request Nov 21, 2024

test: reduce test-esm-loader-hooks-inspect-wait flakiness

b6e5ba5

Refs: nodejs#51560 PR-URL: nodejs#54827 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>

lpinca added a commit to lpinca/node that referenced this pull request Nov 28, 2024

test: deflake test-esm-loader-hooks-inspect-brk

e83cba9

Refs: nodejs#54827 Refs: nodejs#51560

lpinca mentioned this pull request Nov 28, 2024

test: deflake test-esm-loader-hooks-inspect-brk #56050

Merged

nodejs-github-bot pushed a commit that referenced this pull request Dec 19, 2024

test: deflake test-esm-loader-hooks-inspect-brk

990497c

Refs: #54827 Refs: #51560 PR-URL: #56050 Reviewed-By: Antoine du Hamel <[email protected]>

aduh95 pushed a commit that referenced this pull request Jan 2, 2025

test: deflake test-esm-loader-hooks-inspect-brk

7819bfe

Refs: #54827 Refs: #51560 PR-URL: #56050 Reviewed-By: Antoine du Hamel <[email protected]>

aduh95 pushed a commit that referenced this pull request Jan 31, 2025

test: deflake test-esm-loader-hooks-inspect-brk

fa56ab2

Refs: #54827 Refs: #51560 PR-URL: #56050 Reviewed-By: Antoine du Hamel <[email protected]>

marco-ippolito pushed a commit that referenced this pull request Mar 6, 2025

test: deflake test-esm-loader-hooks-inspect-brk

81bae2e

Refs: #54827 Refs: #51560 PR-URL: #56050 Reviewed-By: Antoine du Hamel <[email protected]>

marco-ippolito pushed a commit that referenced this pull request Mar 6, 2025

test: deflake test-esm-loader-hooks-inspect-brk

209e62d

Refs: #54827 Refs: #51560 PR-URL: #56050 Reviewed-By: Antoine du Hamel <[email protected]>

marco-ippolito pushed a commit that referenced this pull request Mar 6, 2025

test: deflake test-esm-loader-hooks-inspect-brk

b4b26e9

Refs: #54827 Refs: #51560 PR-URL: #56050 Reviewed-By: Antoine du Hamel <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

inspector: add NodeRuntime.waitingForDebugger event #51560

inspector: add NodeRuntime.waitingForDebugger event #51560

Uh oh!

mmarchini commented Jan 24, 2024

Uh oh!

nodejs-github-bot commented Jan 24, 2024

Uh oh!

MoLow commented Jan 25, 2024

Uh oh!

koko1928 left a comment

Uh oh!

nodejs-github-bot commented Jan 26, 2024

Uh oh!

nodejs-github-bot commented Feb 3, 2024

Uh oh!

lpinca left a comment

Uh oh!

joyeecheung commented Feb 23, 2024

Uh oh!

nodejs-github-bot commented Feb 23, 2024

Uh oh!

This comment was marked as abuse.

Uh oh!

nodejs-github-bot commented Feb 23, 2024

Uh oh!

nodejs-github-bot commented Feb 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Uh oh!

inspector: add NodeRuntime.waitingForDebugger event #51560

inspector: add NodeRuntime.waitingForDebugger event #51560

Uh oh!

Conversation

mmarchini commented Jan 24, 2024

Uh oh!

nodejs-github-bot commented Jan 24, 2024

Uh oh!

MoLow commented Jan 25, 2024

Uh oh!

koko1928 left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jan 26, 2024

Uh oh!

nodejs-github-bot commented Feb 3, 2024

Uh oh!

lpinca left a comment

Choose a reason for hiding this comment

Uh oh!

joyeecheung commented Feb 23, 2024

Uh oh!

nodejs-github-bot commented Feb 23, 2024

Uh oh!

This comment was marked as abuse.

Uh oh!

nodejs-github-bot commented Feb 23, 2024

Uh oh!

nodejs-github-bot commented Feb 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants