Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

Bi-directional communication with processes #25

Closed
joshwcomeau opened this issue Jun 29, 2018 · 8 comments
Closed

Bi-directional communication with processes #25

joshwcomeau opened this issue Jun 29, 2018 · 8 comments
Labels
help wanted Extra attention is needed top priority Upcoming features of top priority upcoming feature New feature or request

Comments

@joshwcomeau
Copy link
Owner

joshwcomeau commented Jun 29, 2018

A big part of Guppy is the ability to run tasks (AKA NPM scripts).

For most tasks, this is fine; a command is sent, and then output is logged to the screen.

The CRA "eject" script, however, has a warning prompt that requires the user to enter "yes" or "no". In our case this prompt is unnecessary (since the UI has its own prompt), but we need some way to communicate with the process after it's started.

The CRA "test" script, by default, runs in "watch" mode, which means you can enter commands to rerun part of all of the tests. I'd love a future where we have a powerful, easy-to-use module that allows for precise test control, but this requires being able to send arbitrary commands at arbitrary times to the running process

This has been surprisingly hard to solve for me, due to some issues with Electron. The standard approach is to use childProcess.fork; I ran into some issues when trying to use it though (unfortunately I don't remember what they were D:)

We're currently hacking around this in task.middleware.js by simply pre-emptively supplying the command with the pipe operator:

      const command =
        project.type === 'create-react-app' && name === 'eject'
          ? 'echo yes | npm'
          : 'npm';

This is clearly a hack, and it's problematic because it doesn't scale. It would be great to have proper bi-directional communication, for the reasons mentioned above.

This is also a problem because Windows doesn't support echo.

I'd say this is probably the biggest priority issue (as it's blocking Windows support, which is extremely important). If anyone wants to help solve this, I would super duper appreciate it!!

@joshwcomeau joshwcomeau added top priority Upcoming features of top priority upcoming feature New feature or request labels Jun 30, 2018
@mischnic
Copy link

mischnic commented Jul 14, 2018

According to this SO answer it should work on windows as well? (not tested)

But there is another "old school" way to do it that was used back in the day when commands did not have options to suppress confirmation messages. Simply ECHO the needed response and pipe the value into the command.

echo y|rmdir /s modules


This is clearly a hack

Well, Unix even has a command for this specific usecase:

YES(1)                    BSD General Commands Manual                   YES(1)

NAME
     yes -- be repetitively affirmative

SYNOPSIS
     yes [expletive]

DESCRIPTION
     yes outputs expletive, or, by default, ``y'', forever.

HISTORY
     The yes command appeared in 4.0BSD.

4th Berkeley Distribution        June 6, 1993        4th Berkeley Distribution

@superhawk610
Copy link
Collaborator

I'm not near my Mac right now to test this, but couldn't you use childProcess.stdin.write() when prompted? Something like:

// src/services/create-project.service.js
process.stdout.on('data', (data) => {
  if (data === 'Are you sure?') {
    process.stdin.write('y\n');
  }
  onStatusUpdate(data);
});

@joshwcomeau
Copy link
Owner Author

joshwcomeau commented Jul 15, 2018

Thanks for the feedback, @mischnic and @superhawk610!

I wasn't super clear in my original issue, but ideally I'd like to find a way to fix the hack, rather than find a way to make the hack work on Windows as well. The reason for this is that I'd love to have an interactive testing module, where users can click buttons to re-run tests. This would require true bi-directional communication, not just trying to pre-emptively answer a prompt.

That said, this is currently blocking Windows support, which is more important than a new testing module. So I'm gonna try both of your suggestions and see if we can do this in a couple steps (top-priority bandaid to unblock Windows, and then later a rearchitecture of how process communication works)

@joshwcomeau joshwcomeau added the help wanted Extra attention is needed label Jul 15, 2018
@j-f1
Copy link
Collaborator

j-f1 commented Jul 15, 2018

@superhawk610’s suggestion would also work for a test runner UI — you just need to parse the stdout to ensure Jest is ready for input, then run childProcess.stdin.write(input).

@joshwcomeau
Copy link
Owner Author

@superhawk610’s suggestion would also work for a test runner UI — you just need to parse the stdout to ensure Jest is ready for input, then run childProcess.stdin.write(input).

Ohh, my bad! I'm sorry, I read through that comment too quickly.

So I tried this, and it doesn't look like it works; it appears to be writing to stdin (and forwarding to stdout) without being "processed" by the task. It gets stuck in a loop where it just keeps adding ys:

screen shot 2018-07-17 at 8 56 38 am

Interestingly, when I abort and re-run the task, it does appear to send the message (the project ejects, although it crashes for unknown reasons).

If anyone else wants to try this, the necessary changes were in task.middleware.js:

# line 145
      const command = 'npm';
      // project.type === 'create-react-app' && name === 'eject'
      //   ? 'echo yes | npm'
      //   : 'npm';

# line 166
      child.stdout.on('data', data => {
        console.log('DATA', data.toString());
        if (data.includes('Are you sure')) {
          child.stdin.write('y');
        }
        next(receiveDataFromTaskExecution(task, data.toString()));
      });

Another facet to this: I'm struggling to remember, but I believe one potential is that tasks are run in shell mode; for bi-directional communication with child processes, you're supposed to use fork, but fork doesn't work with shell mode.

The reason we use shell is because of an issue with child processes not otherwise inheriting the environment variables. On MacOS, at least, this means that PATH is unset, which means literally nothing works (basic commands like which fail with ENOENT).

We got a potential solution to that problem with this StackOverflow reply yesterday; I did a quick test, and it appears to work, although we'd need to figure out if either of the suggested solutions can be adapted for Windows.

@superhawk610
Copy link
Collaborator

superhawk610 commented Jul 17, 2018

@joshwcomeau you have to terminate each input command with a newline - think about it like physically using a TTY, you have to press Enter for your command to be processed and not just output to stdout.

child.stdin.write('y\n');

@joshwcomeau
Copy link
Owner Author

@joshwcomeau you have to terminate each input command with a newline - think about it like physically using a TTY, you have to press Enter for your command to be processed and not just output to stdout.

Ohhhh, of course. D'oh 😅

@joshwcomeau
Copy link
Owner Author

This is a solved issue :D

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Extra attention is needed top priority Upcoming features of top priority upcoming feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants