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

Promise-based IPC #1053

Closed
ehmicky opened this issue May 13, 2024 · 2 comments · Fixed by #1059
Closed

Promise-based IPC #1053

ehmicky opened this issue May 13, 2024 · 2 comments · Fixed by #1059

Comments

@ehmicky
Copy link
Collaborator

ehmicky commented May 13, 2024

Current API (callbacks)

Currently, to use Node.js IPC, one must use callbacks.

// parent.js
const subprocess = execaNode('child.js');

subprocess.send(sentMessage, error => {
  // ...
});

subprocess.once('message', oneMessage => {
  // ...
});

subprocess.on('message', eachMessage => {
  // ...
});
// child.js
import process from 'node:process';

process.send(sentMessage, error => {
  // ...
}); 

process.once('message', oneMessage => {
  // ...
});

process.on('message', eachMessage => {
  // ...
});

New API (promises)

To make it work better with async/await, provide with better stack traces and avoid uncaught exceptions, we should expose that API using promises instead.

// parent.js
const subprocess = execaNode('child.js');

await subprocess.ipc.send(sentMessage);

const oneMessage = await subprocess.ipc.receive();

for await (const eachMessage of subprocess.ipc.iterable()) {
  // ...
};
// child.js
import {ipc} from 'execa';

await ipc.send(sentMessage); 

const oneMessage = await ipc.receive();

for await (const eachMessage of ipc.iterable()) {
  // ...
};

Additional pros

I have just looked into Node.js/libuv source code for the IPC feature to check for edge cases. On top of just providing with a promise-based API instead of callbacks, this would also benefit the additional pros.

Graceful exit

When using process.on('message') or subprocess.on('message'), the process is kept alive. My guess is that most users probably solve this by calling either:

  • process.exit(): this is bad because it interrupts any ongoing logic.
  • process.disconnect(): this is bad because it prevents calling process.on('message') later. In other words, it is not possible to stop listening then restart later.

The proper way is to remove the message event listener, which we would automatically do.

(Note: under the hood, Node.js listens for addListener and removeListener events on the process.)

Error handling

It's a little hard right now for users to both use the IPC callbacks API, while at the same time await the Execa promise in case the subprocess failed. Using a promise-based API would ensure no subprocess failure is creating unhandled promise rejections.

Backpressure

subprocess.send() returns false when more than ~350KB has been buffered but not sent yet. This works like streams highWaterMark, for backpressure. I.e. the best practice would be for users to stop sending data when false is returned.

I am guessing most users might not do this. Our API would automatically provide with backpressure, since the buffer is always emptied when the subprocess.send() callback is fired. So awaiting the .send() promise would be enough to ensure the IPC channel is not under pressure.

Better validation

If the user forgot to use the ipc: true option, we can provide with a nice error message. Right now, they might be wondering why process.send crashes when it is undefined.

Simplicity

With that API, users should never have to use process.disconnect(), process.on('disconnect'), process.connected or process.channel. This is automatically handled for them.

Backward compatibility

The callback-based API would still be available, but undocumented.


What do you think @sindresorhus?

@ehmicky ehmicky mentioned this issue May 13, 2024
@sindresorhus
Copy link
Owner

That looks like a better API indeed 👍

@ehmicky
Copy link
Collaborator Author

ehmicky commented May 15, 2024

I started working on this and I realized I kept forgetting to type ipc. If I do, users might do as well. I am now using the following method names instead. If the new names don't work, please let me know, and I'll rename them.

// parent.js
const subprocess = execaNode('child.js');

await subprocess.sendMessage(sentMessage);

const oneMessage = await subprocess.getOneMessage();

for await (const eachMessage of subprocess.getEachMessage()) {
  // ...
};
// child.js
import {sendMessage, getOneMessage, getEachMessage} from 'execa';

await sendMessage(sentMessage); 

const oneMessage = await getOneMessage();

for await (const eachMessage of getEachMessage()) {
  // ...
};

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