Propose changes to the Deno.run() API

[bartlomieju]

Given unintuitive way of Deno.run() API usage I think we should consider changing that API for Deno 2.0.

Tokio provides a great abstraction for running subprocess by the means of Command and Child APIs
especially if user wants to just run process to completion or run it to completion while collecting output.

Currently Deno APIs requires users to handle both situations manually; and I believe it's worth considering to add some helper methods.

Hereby I propose to align Deno's subprocess APIs to follow Tokio's API as closely as possible.

// run process to completion
const command = new Deno.Command({
    cmd: ["ls"],
});
const status = await command.status();

// run process to completion, collecting output (will configure stdout/stderr to be piped automatically)
const command = new Deno.Command({
    cmd: ["ls"],
});
const { status, stdout, stderr } = await command.output();

// spawn a subprocess and return a handle to it
const command = new Deno.Command({
    cmd: ["ls"],
});
const subprocess = command.spawn();

// wait for subprocess to complete
const status = await subprocess.wait();

// check if subprocess has completed
const maybeStatus = subprocess.tryWait(); // rename to status() ?

// wait for subproces to complete and collect output
const { status, stdout, stderr } = await subprocess.waitWithOutput();

// force subprocess to exit
await subprocess.kill();

With the proposed changes to the API all appropriate resources used to facilitate subprocess
would be automatically cleaned up (especially stdio resources), meaning that for the most popular
use cases users wouldn't have to remember to call Deno.close() on the resources.

For more advanced use cases (eg. streaming data from stdout/stderr) there wouldn't be much difference
in the API usability besides changing API names.

[caspervonb]

You used a string for cmd here as option; so I assume this means a command with args can be created like this?

const cmd = new Command({
  cmd: "ls",
  args: ["-A"],
  env: { ... },
);

And just the options in general; since Rust's std and tokio use a builder pattern how do we do the options? still an options bag?

Also, do we keep Deno.run around as a convenience function (we don't have many but we do have some) to create a command and spawn a subprocess with said command?

[caspervonb]

Also the program (cmd) always has to be present right, so why not make that positional? Think it was a mistake to stick that in the options bag for Deno.run and I'd like to fix it if we're revisiting 😄

[bartlomieju]

You used a string for cmd here as option; so I assume this means a command with args can be created like this?

const cmd = new Command({
  cmd: "ls",
  args: ["-A"],
  env: { ... },
);

And just the options in general; since Rust's std and tokio use a builder pattern how do we do the options? still an options bag?

Also, do we keep Deno.run around as a convenience function (we don't have many but we do have some) to create a command and spawn a subprocess with said command?

Actually not, I think the current cmd is fine. I'll update the examples.

[lucacasonato]

// check if subprocess has completed
const maybeStatus = subprocess.tryWait();

I feel like this should be subprocess.status() and return Status | null. tryWait doesn't really make much sense IMO.

[nayeemrmn]

// check if subprocess has completed
const maybeStatus = subprocess.tryWait();

We should avoid this one; I think it's an antipattern in JS. It can be done with .then() closures instead.

[bartlomieju]

// check if subprocess has completed
const maybeStatus = subprocess.tryWait();

We should avoid this one; I think it's an antipattern in JS. It can be done with .then() closures instead.

Not sure I understand, the crux of this method is that it's sync and won't wait for process completion, currently there's no way in Deno API to check if process has already finished.

// check if subprocess has completed
const maybeStatus = subprocess.tryWait();

I feel like this should be subprocess.status() and return Status | null. tryWait doesn't really make much sense IMO.

What's wrong with tryWait? One of premises for this change is to follow Tokio's API as closely as possible including naming of APIs.

[lucacasonato]

We should avoid this one; I think it's an antipattern in JS. It can be done with .then() closures instead.

Yeah - agree. Polling is very much an anti-pattern.

[nayeemrmn]

Not sure I understand, the crux of this method is that it's sync and won't wait for process completion, currently there's no way in Deno API to check if process has already finished.

  const maybeStatus = null;
  subprocess.wait().then(status => {
    maybeStatus = status;
  });
  
  // check if subprocess has completed
  if (maybeStatus != null) { ... }

[bartlomieju]

Not sure I understand, the crux of this method is that it's sync and won't wait for process completion, currently there's no way in Deno API to check if process has already finished.

  const maybeStatus = null;
  subprocess.status().then(status => {
    maybeStatus = status;
  });
  
  // check if subprocess has completed
  if (maybeStatus != null) { ... }

@nayeemrmn that's not really equivalent - calling Child::wait in Tokio API closes stdin handle automatically to avoid a deadlock, while Child::try_wait does not close stdin.

[nayeemrmn]

calling Child::wait in Tokio API closes stdin handle automatically to avoid a deadlock

So is there no way to asynchronously listen for subprocess exit without closing stdin?

[bartlomieju]

calling Child::wait in Tokio API closes stdin handle automatically to avoid a deadlock

So is there no way to asynchronously listen for subprocess exit without closing stdin?

I believe so, at least that what Child docs say.

[rivy]

@bartlomieju ,

During the re-architecture of the API, I hope you'll consider adding some way to bypass command and argument processing (quoting, etc).

Currently, it's impossible to execute certain valid Windows command lines in Deno. For example, cmd /d/c "deno eval -T "console.log('a string')"" is a valid executable command line ...

C:>cmd /d/c "deno eval -T "console.log('a string')""
Check file:///C:/Users/Roy/OneDrive/Projects/deno/$deno$eval.ts
a string

when implemented via Deno.run(...) ...

await Deno.run({cmd: ['cmd', '/d/c', '"deno eval -T "console.log(\'a string\')""']}).status();
// => `The system cannot find the path specified.`

or ...

await Deno.run({cmd: ['cmd', '/d/c', 'deno eval -T "console.log(\'a string\')"']}).status();
// => `error: Unterminated string constant at file:///C:/Users/Roy/OneDrive/Projects/deno/$deno$eval.ts:1:0`

It's not possible to construct this command line because the inner double quotes are escaped, which, in turn, breaks the interpretation by cmd.

In part, this is because Deno is implemented in Rust, which has the same (unresolved) problem with process::Command. Rust can, however, drop down to the Win32 CreateProcess which gives an escape hatch, but that isn't possible (or maybe even desirable) in Deno.

[piscisaureus]

'cmd', '/d/c', '"deno eval -T "console.log(\'a string\')""']}).status();

Note that /d and /c should be separate arguments.

[rivy]

'cmd', '/d/c', '"deno eval -T "console.log(\'a string\')""']}).status();

Note that /d and /c should be separate arguments.

No. They can be separate but that is not required (as shown by the example).

[wperron]

While we're looking at this API, I really like Golang's exec package -- Specifically, the Command struct's properties for i/o. It makes it super easy to hook a subprocess's input and output to the rest of the parent process.

type Command struct {
    // ...
    Stdin io.Reader
    Stdout io.Writer
    Stderr io.Writer
}

[andykais]

Just wanted to make sure that this issue #5501 stays a part of the discussion. Having a subprocess.detach() method will open up a lot more use cases for deno scripting

[ry]

Rather unorganized notes from last subprocess meeting https://gist.github.com/569dc7d20e1841a84cf89e2bd7dc3f6c

[piscisaureus]

Bert/Luca/Ben/Ryan agreed that it would be nice if no zombies are left after deno exits, unless the user opts in to detached semantics.

[andykais]

is that in conflict with the resolution of #8772?

this behaviour [calling Deno.exit() will stop the deno process, but will not stop any subprocesses running inside it] aligns to Node.js and further research indicating that this isn't something that Deno should do specifically.

[guest271314]

When the processes is killed when the parent is killed it makes it difficult to use a Native Messaging host to start and stop a local server without keeping the Native Messaging host process open.

This sends a single message to the Deno Native Messaging host

  chrome.runtime.sendNativeMessage('native_messaging_espeakng'
  , {}, (nativeMessage) => console.log({nativeMessage}))

Unfortunately when the parent Native Messaging host exists the local server exists - unless I am missing some way to keep the child process open when the parent process closes.

  let message = '';
  let process = Deno.run({
    cmd: [`pgrep`, `-f`, `deno_server.js`],
    stdout: 'piped',
    stderr: 'piped',
  });

  let rawOutput = await process.output();

  if (rawOutput.length) {
    process.close();
    process = Deno.run({
      cmd: [`pkill`, `-f`, `./deno_server.js`],
      stdout: 'piped',
      stderr: 'piped',
    });

    await process.status();
    process.close();
    message = '"Local server off."';
  } else {
    process = Deno.run({
      cmd: ['./deno_server.js'],
      stdout: 'piped',
      stderr: 'piped',
    });
    await Promise.race([null, process.status()]);
    message = '"Local server on."';
  }

[jespertheend]

Not sure how high level this api is meant to be, but would it be possible to allow full commands as strings? I.e. new Deno.Command("git status"). At the moment this throws because it expects the arguments to be separate.

9 out of 10 times when I need subprocess functionality, it's to just run a simple command and then get its output as text. Something like:

const currentBranch = await runCmd("git branch --show-current");

It would be nice to have the new api be a bit closer to this. Maybe something like:

const cmd = new Deno.Command("git branch --show-current");
const currentBranch = await cmd.text();

Ideally cmd.text() would check for a zero status code and throw otherwise.

Not sure if this is too high level, perhaps this is more suitable for deno_std.

[crowlKats]

I am not sure i understand what the text() metho would do

[MierenManz]

The idea of it is. If status code is 0 return stdout as string or throw if nonzero

[crowlKats]

ok, then i would say both of these suggestions would fit more for std