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.

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

--watch mode disables IPC communication between a spawning process and the actual child process #50880

Closed
znewsham opened this issue Nov 23, 2023 · 3 comments · Fixed by #50890

Comments

@znewsham
Copy link
Contributor

Version

v18.17.1

Platform

Linux zacknewsham-xps 6.2.0-33-generic #33~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu Sep 7 10:33:52 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

This isn't quite a bug, nor is it quite a feature request, but somewhere in between when using watch mode.

in parent.js

import { spawn } from 'child_process';
spawn(
  'node',
  '--watch',
  'child.js'
);

child.sendMessage({ type: "whatever" }, console.log);

in child.js

process.on("message", console.log);

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

I expect to be able to communicate with a spawned child process, even when using --watch.

What do you see instead?

Because the --watch mode uses IPC itself (to listen to messages from the child process) it doesn't pass through messages from the child to the parent, or the parent to the child.

Additional information

As I said, this isn't quite a bug - but not quite a feature request either. I believe the fix is relatively simple, if the process has IPC enabled, then pass through messages (that aren't watch related) from the child -> parent and all messages from parent -> child

@znewsham
Copy link
Contributor Author

Ok, it turns out the fix is extremely simple, I'm happy to open a PR for this, I'm just not sure how to go about testing it. I think it's probably fine to require the parent process filter out unrelated (e.g., messages with watch: prefixed keys) IPC messages.

in 'lib/internal/files_watcher.js', add:

  #setupIPC(child) {
    process.on("message", message => child.send(message));
    child.on("message", message => process.send(message));
  }

then modify constructor to have:

  #wantsPassthroughIPC = false;

  constructor({ throttle = 500, mode = 'filter' } = kEmptyObject) {
    super();
    this.#wantsPassthroughIPC = !!process.send;

then modify watchChildProcessModules to have:

  watchChildProcessModules(child, key = null) {
    if (this.#wantsPassthroughIPC) {
      this.#setupIPC(child);
    }

@znewsham
Copy link
Contributor Author

@MoLow not sure what your thoughts are on this

@MoLow
Copy link
Member

MoLow commented Nov 23, 2023

Sound like a great solution,
for testing I would just add an extra case to https://github.com/nodejs/node/blob/main/test/sequential/test-watch-mode.mjs
with a fixture printing messages, validating it receives messages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants