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.

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

inspector: enable Inspector JS API in workers #22769

Merged
merged 0 commits into from Sep 17, 2018

Conversation

Projects
None yet
7 participants
@eugeneo
Contributor

eugeneo commented Sep 9, 2018

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
@eugeneo

This comment has been minimized.

Show comment
Hide comment
@eugeneo

eugeneo Sep 9, 2018

Contributor

This commit was factored out from #21364 and mostly deals with tests running in a worker.

Contributor

eugeneo commented Sep 9, 2018

This commit was factored out from #21364 and mostly deals with tests running in a worker.

@eugeneo eugeneo added the inspector label Sep 9, 2018

@eugeneo

This comment has been minimized.

Show comment
Hide comment
Contributor

eugeneo commented Sep 9, 2018

@addaleax addaleax added the worker label Sep 9, 2018

@vsemozhetbyt

This comment has been minimized.

Show comment
Hide comment
@vsemozhetbyt

vsemozhetbyt Sep 10, 2018

Member

Should the exported common.skipIfWorker() be documented?

Member

vsemozhetbyt commented Sep 10, 2018

Should the exported common.skipIfWorker() be documented?

@eugeneo

This comment has been minimized.

Show comment
Hide comment
@eugeneo

eugeneo Sep 10, 2018

Contributor

@vsemozhetbyt

Should the exported common.skipIfWorker() be documented?

Done.

Contributor

eugeneo commented Sep 10, 2018

@vsemozhetbyt

Should the exported common.skipIfWorker() be documented?

Done.

eugeneo added a commit to eugeneo/node that referenced this pull request Sep 11, 2018

inspector: enable Inspector JS API in workers
PR-URL: nodejs#22769
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@eugeneo

This comment has been minimized.

Show comment
Hide comment
Contributor

eugeneo commented Sep 11, 2018

@eugeneo

This comment has been minimized.

Show comment
Hide comment
@eugeneo

eugeneo Sep 12, 2018

Contributor

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

Previous run did not show any failures obviously caused by this change.

Contributor

eugeneo commented Sep 12, 2018

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

Previous run did not show any failures obviously caused by this change.

@eugeneo

This comment has been minimized.

Show comment
Hide comment
@eugeneo

eugeneo Sep 12, 2018

Contributor

AIX failure is the bot running out of drive space. Still, would prefer to wait for Windows CI.

Contributor

eugeneo commented Sep 12, 2018

AIX failure is the bot running out of drive space. Still, would prefer to wait for Windows CI.

@joaocgreis

This comment has been minimized.

Show comment
Hide comment
@joaocgreis

joaocgreis Sep 15, 2018

Member

Last CI run was not successful for unrelated reasons (nodejs/build#1495).
Resumed: https://ci.nodejs.org/job/node-test-pull-request/17196/

Member

joaocgreis commented Sep 15, 2018

Last CI run was not successful for unrelated reasons (nodejs/build#1495).
Resumed: https://ci.nodejs.org/job/node-test-pull-request/17196/

@eugeneo

This comment has been minimized.

Show comment
Hide comment
@eugeneo

eugeneo Sep 17, 2018

Contributor

Last CI has a know flaky test and a Git failure on a bot. I will land it now.

Contributor

eugeneo commented Sep 17, 2018

Last CI has a know flaky test and a Git failure on a bot. I will land it now.

@eugeneo eugeneo closed this Sep 17, 2018

@eugeneo eugeneo deleted the eugeneo:js_bindings_in_worker branch Sep 17, 2018

@eugeneo eugeneo merged commit ab5f789 into nodejs:master Sep 17, 2018

1 check was pending

continuous-integration/travis-ci/push The Travis CI build is in progress
Details
@eugeneo

This comment has been minimized.

Show comment
Hide comment
@eugeneo

eugeneo Sep 17, 2018

Contributor

Landed as eugeneo@ab5f789

Contributor

eugeneo commented Sep 17, 2018

Landed as eugeneo@ab5f789

addaleax added a commit to addaleax/node that referenced this pull request Sep 18, 2018

worker: only stop inspector if started
This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: nodejs#22769

@addaleax addaleax referenced this pull request Sep 18, 2018

Closed

worker: only stop inspector if started #22927

2 of 2 tasks complete

addaleax added a commit to addaleax/node that referenced this pull request Sep 18, 2018

worker: only stop inspector if started
This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: nodejs#22769

targos added a commit that referenced this pull request Sep 18, 2018

inspector: enable Inspector JS API in workers
PR-URL: #22769
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Trott added a commit to Trott/io.js that referenced this pull request Sep 19, 2018

worker: only stop inspector if started
This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: nodejs#22769

PR-URL: nodejs#22927
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>

targos added a commit that referenced this pull request Sep 19, 2018

worker: only stop inspector if started
This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: #22769

PR-URL: #22927
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>

targos added a commit that referenced this pull request Sep 19, 2018

inspector: enable Inspector JS API in workers
PR-URL: #22769
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

targos added a commit that referenced this pull request Sep 19, 2018

worker: only stop inspector if started
This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: #22769

PR-URL: #22927
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>

targos added a commit that referenced this pull request Sep 20, 2018

inspector: enable Inspector JS API in workers
PR-URL: #22769
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

targos added a commit that referenced this pull request Sep 20, 2018

worker: only stop inspector if started
This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: #22769

PR-URL: #22927
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
@targos

This comment has been minimized.

Show comment
Hide comment
@targos

targos Sep 20, 2018

Member

Depends on #21875. Marking dont-land-on-v10.x for now.

Member

targos commented Sep 20, 2018

Depends on #21875. Marking dont-land-on-v10.x for now.

@targos targos referenced this pull request Sep 20, 2018

Closed

fs: implement recursive (mkdirp) functionality #21875

4 of 4 tasks complete
@eugeneo

This comment has been minimized.

Show comment
Hide comment
@eugeneo

eugeneo Sep 20, 2018

Contributor

What would be the dependency? This backport is needed for #22954

Contributor

eugeneo commented Sep 20, 2018

What would be the dependency? This backport is needed for #22954

@targos

This comment has been minimized.

Show comment
Hide comment
@targos

targos Sep 20, 2018

Member

@eugeneo I didn't have time to check. It may not depend directly on that PR but on another PR that depends on it.

Member

targos commented Sep 20, 2018

@eugeneo I didn't have time to check. It may not depend directly on that PR but on another PR that depends on it.

@targos targos added this to Don't land (for now) in v10.x Sep 23, 2018

@targos targos removed this from Don't land (for now) in v10.x Sep 25, 2018

targos added a commit that referenced this pull request Sep 25, 2018

inspector: enable Inspector JS API in workers
PR-URL: #22769
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

targos added a commit that referenced this pull request Sep 25, 2018

worker: only stop inspector if started
This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: #22769

PR-URL: #22927
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>

@targos targos referenced this pull request Oct 7, 2018

Merged

Release proposal: v10.12.0 #23313

targos added a commit that referenced this pull request Oct 10, 2018

2018-10-10, Version 10.12.0 (Current)
Notable changes:

* assert
  * The diff output is now a tiny bit improved by sorting object
    properties when inspecting the values that are compared with each
    other. #22788
* cli
  * The options parser now normalizes `_` to `-` in all multi-word
    command-line flags, e.g. `--no_warnings` has the same effect as
    `--no-warnings`. #23020
  * Added bash completion for the `node` binary. To generate a bash
    completion script, run `node --completion-bash`. The output can be
    saved to a file which can be sourced to enable completion.
    #20713
* crypto
  * Added support for PEM-level encryption.
    #23151
  * Added an API asymmetric key pair generation. The new methods
    `crypto.generateKeyPair` and `crypto.generateKeyPairSync` can be
    used to generate public and private key pairs. The API supports
    RSA, DSA and EC and a variety of key encodings (both PEM and DER).
    #22660
* fs
  * Added a `recursive` option to `fs.mkdir` and `fs.mkdirSync`. If
    this option is set to true, non-existing parent folders will be
    automatically created. #21875
* http2
  * Added a `'ping'` event to `Http2Session` that is emitted whenever a
    non-ack `PING` is received.
    #23009
  * Added support for the `ORIGIN` frame.
    #22956
  * Updated nghttp2 to 1.34.0. This adds RFC 8441 extended connect
    protocol support to allow use of WebSockets over HTTP/2.
    #23284
* module
  * Added `module.createRequireFromPath(filename)`. This new method can
    be used to create a custom require function that will resolve
    modules relative to the filename path.
    #19360
* process
  * Added a `'multipleResolves'` process event that is emitted whenever
    a `Promise` is attempted to be resolved multiple times, e.g. if the
    `resolve` and `reject` functions are both called in a `Promise`
    executor. #22218
* url
  * Added `url.fileURLToPath(url)` and `url.pathToFileURL(path)`. These
    methods can be used to correctly convert between file: URLs and
    absolute paths. #22506
* util
  * Added the `sorted` option to `util.inspect()`. If set to `true`,
    all properties of an object and Set and Map entries will be sorted
    in the returned string. If set to a function, it is used as a
    compare function. #22788
  * The `util.instpect.custom` symbol is now defined in the global
    symbol registry as `Symbol.for('nodejs.util.inspect.custom')`.
    #20857
  * Added support for `BigInt` numbers in `util.format()`.
    #22097
* V8 API
  * A number of V8 C++ APIs have been marked as deprecated since they
    have been removed in the upstream repository. Replacement APIs
    are added where necessary. #23159
* Windows
  * The Windows msi installer now provides an option to automatically
    install the tools required to build native modules.
    #22645
* Workers
  * Debugging support for Workers using the DevTools protocol has been
    implemented. #21364
  * The public `inspector` module is now enabled in Workers.
    #22769
* Added new collaborators:
  * digitalinfinity - Hitesh Kanwathirtha

PR-URL: #23313

targos added a commit that referenced this pull request Oct 10, 2018

2018-10-10, Version 10.12.0 (Current)
Notable changes:

* assert
  * The diff output is now a tiny bit improved by sorting object
    properties when inspecting the values that are compared with each
    other. #22788
* cli
  * The options parser now normalizes `_` to `-` in all multi-word
    command-line flags, e.g. `--no_warnings` has the same effect as
    `--no-warnings`. #23020
  * Added bash completion for the `node` binary. To generate a bash
    completion script, run `node --completion-bash`. The output can be
    saved to a file which can be sourced to enable completion.
    #20713
* crypto
  * Added support for PEM-level encryption.
    #23151
  * Added an API asymmetric key pair generation. The new methods
    `crypto.generateKeyPair` and `crypto.generateKeyPairSync` can be
    used to generate public and private key pairs. The API supports
    RSA, DSA and EC and a variety of key encodings (both PEM and DER).
    #22660
* fs
  * Added a `recursive` option to `fs.mkdir` and `fs.mkdirSync`. If
    this option is set to true, non-existing parent folders will be
    automatically created. #21875
* http2
  * Added a `'ping'` event to `Http2Session` that is emitted whenever a
    non-ack `PING` is received.
    #23009
  * Added support for the `ORIGIN` frame.
    #22956
  * Updated nghttp2 to 1.34.0. This adds RFC 8441 extended connect
    protocol support to allow use of WebSockets over HTTP/2.
    #23284
* module
  * Added `module.createRequireFromPath(filename)`. This new method can
    be used to create a custom require function that will resolve
    modules relative to the filename path.
    #19360
* process
  * Added a `'multipleResolves'` process event that is emitted whenever
    a `Promise` is attempted to be resolved multiple times, e.g. if the
    `resolve` and `reject` functions are both called in a `Promise`
    executor. #22218
* url
  * Added `url.fileURLToPath(url)` and `url.pathToFileURL(path)`. These
    methods can be used to correctly convert between file: URLs and
    absolute paths. #22506
* util
  * Added the `sorted` option to `util.inspect()`. If set to `true`,
    all properties of an object and Set and Map entries will be sorted
    in the returned string. If set to a function, it is used as a
    compare function. #22788
  * The `util.instpect.custom` symbol is now defined in the global
    symbol registry as `Symbol.for('nodejs.util.inspect.custom')`.
    #20857
  * Added support for `BigInt` numbers in `util.format()`.
    #22097
* V8 API
  * A number of V8 C++ APIs have been marked as deprecated since they
    have been removed in the upstream repository. Replacement APIs
    are added where necessary. #23159
* Windows
  * The Windows msi installer now provides an option to automatically
    install the tools required to build native modules.
    #22645
* Workers
  * Debugging support for Workers using the DevTools protocol has been
    implemented. #21364
  * The public `inspector` module is now enabled in Workers.
    #22769
* Added new collaborators:
  * digitalinfinity - Hitesh Kanwathirtha

PR-URL: #23313

targos added a commit that referenced this pull request Oct 10, 2018

2018-10-10, Version 10.12.0 (Current)
Notable changes:

* assert
  * The diff output is now a tiny bit improved by sorting object
    properties when inspecting the values that are compared with each
    other. #22788
* cli
  * The options parser now normalizes `_` to `-` in all multi-word
    command-line flags, e.g. `--no_warnings` has the same effect as
    `--no-warnings`. #23020
  * Added bash completion for the `node` binary. To generate a bash
    completion script, run `node --completion-bash`. The output can be
    saved to a file which can be sourced to enable completion.
    #20713
* crypto
  * Added support for PEM-level encryption.
    #23151
  * Added an API asymmetric key pair generation. The new methods
    `crypto.generateKeyPair` and `crypto.generateKeyPairSync` can be
    used to generate public and private key pairs. The API supports
    RSA, DSA and EC and a variety of key encodings (both PEM and DER).
    #22660
* fs
  * Added a `recursive` option to `fs.mkdir` and `fs.mkdirSync`. If
    this option is set to true, non-existing parent folders will be
    automatically created. #21875
* http2
  * Added a `'ping'` event to `Http2Session` that is emitted whenever a
    non-ack `PING` is received.
    #23009
  * Added support for the `ORIGIN` frame.
    #22956
  * Updated nghttp2 to 1.34.0. This adds RFC 8441 extended connect
    protocol support to allow use of WebSockets over HTTP/2.
    #23284
* module
  * Added `module.createRequireFromPath(filename)`. This new method can
    be used to create a custom require function that will resolve
    modules relative to the filename path.
    #19360
* process
  * Added a `'multipleResolves'` process event that is emitted whenever
    a `Promise` is attempted to be resolved multiple times, e.g. if the
    `resolve` and `reject` functions are both called in a `Promise`
    executor. #22218
* url
  * Added `url.fileURLToPath(url)` and `url.pathToFileURL(path)`. These
    methods can be used to correctly convert between file: URLs and
    absolute paths. #22506
* util
  * Added the `sorted` option to `util.inspect()`. If set to `true`,
    all properties of an object and Set and Map entries will be sorted
    in the returned string. If set to a function, it is used as a
    compare function. #22788
  * The `util.instpect.custom` symbol is now defined in the global
    symbol registry as `Symbol.for('nodejs.util.inspect.custom')`.
    #20857
  * Added support for `BigInt` numbers in `util.format()`.
    #22097
* V8 API
  * A number of V8 C++ APIs have been marked as deprecated since they
    have been removed in the upstream repository. Replacement APIs
    are added where necessary. #23159
* Windows
  * The Windows msi installer now provides an option to automatically
    install the tools required to build native modules.
    #22645
* Workers
  * Debugging support for Workers using the DevTools protocol has been
    implemented. #21364
  * The public `inspector` module is now enabled in Workers.
    #22769
* Added new collaborators:
  * digitalinfinity - Hitesh Kanwathirtha

PR-URL: #23313
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment