Have to send two SIGINT to kill when prompt is open

[mbleigh]

If you Ctrl-C out of a command while an inquirer prompt is open, it just adds a couple lines of space and then you have to Ctrl-C again to actually exit. I'd expect the calling program to be the one controlling what happens on SIGINT here.

Is this working as intended? If so, is there a way to be notified when Inquirer has trapped a SIGINT?

[SBoudrias]

Inquirer shouldn't trap SIGINT. What's your OS/terminal/node version?

[mbleigh]

Interesting...I tried to create a minimal repro but couldn't reproduce the behavior. I thought I had seen the behavior in bower init but now can't reproduce that either. Feel free to mark as closed, I must be mistaken about the cause of my issue.

[ChristianAlexander]

I ran into this issue recently. Because of Inquirer.js's usage of readline, SIGINT only gets sent to Inquirer, which is fine when Inquirer is in the only running process.

For me, the problem came up when I had a child process running. Since Inquirer does not propagate the signal, it is never exposed to the main process. This essentially removes any ability to listen for any signals with process.on.

The real question is, should SIGINT only go to Inquirer, and be required a second time for the process, or should Inquirer be responsible and ensure that SIGINT is sent properly?

[SBoudrias]

@ChristianAlexander can you link to the Node documentation stating that readlines won't propagate signals to the process?

[ChristianAlexander]

After looking into this further, there is no documentation explicitly stating that signals do not propagate. In fact, manually sending a SIGINT (ex: kill -2 <node process id>) does work.

The problem is that readline itself listens to the TTY directly, so a 'real' SIGINT isn't produced.
See: https://github.com/nodejs/node/blob/master/lib/readline.js#L689-L700

[SBoudrias]

@ChristianAlexander mind opening an issue on Node and linking this issue?

[ChristianAlexander]

I'm unsure if this should be considered an issue at the Node level. Perhaps there could be a case where it wouldn't be desirable to send a SIGINT on ctrl+c in a project directly dependent on readline.

However, I can not come up with a scenario where any Inquirer-based projects would use ctrl+c. Even if such a scenario were to exist, the event wouldn't be exposed to consumers of Inquirer.

If Inquirer doesn't end up sending SIGINTs to the process itself, would it be possible to have Inquirer pass the event along after it is done closing up? If not, could Inquirer send an error object in its callback where results are sent?

[SBoudrias]

I'm unsure if this should be considered an issue at the Node level. Perhaps there could be a case where it wouldn't be desirable to send a SIGINT on ctrl+c in a project directly dependent on readline.

I don't know, but that seems like a conversation worth having with the Node community.

If Inquirer doesn't end up sending SIGINTs to the process itself, would it be possible to have Inquirer pass the event along after it is done closing up? If not, could Inquirer send an error object in its callback where results are sent?

Feel free to send a PR.

[felipenmoura]

So...
I want to capture the ctrl+c to, let's say, output a "good bye" before exiting, killing a socket, and other small things like logs.
But I also want to listen to ctrl+l to clear the console, and if possible, esc!
If inquirer does receive these events somehow, it could have an API so we could have access to them.

[SBoudrias]

@felipenmoura these events are global, you don't need to listen to it on inquirer.

[felipenmoura]

Well, they are not being triggered!
I am using inquirer, with wait, and also using the process.on('SIGINT', ... ), and it is never called!
I am not sure if I am doing something wrong, or if it has something to do with this thread.

[SBoudrias]

@felipenmoura that's what this thread is saying. That's due to the Node.js readline. You guys should open an issue on the Node project.

[dcrockwell]

What about some kind of solution like exposing the readline instance as a workaround?

See this comment from a Node.js Foundation Member: nodejs/node#4758 (comment)

[adamreisnz]

Does anyone have any new insights or workarounds for this issue? It's still present one year on and quite annoying that one has to CTRL-C twice to exit a process where inquirer prompt is active.

Looks like the suggestion as above by @dcrockwell is already present in the code but to no avail:

https://github.com/SBoudrias/Inquirer.js/blob/master/lib/ui/baseUI.js#L21

Further investigation shows that the called onClose handler doesn't really do anything except output a new line:

UI.prototype.onForceClose = function () {
  this.close();
  console.log('');
};

I'll create a PR to fix this.

[welwood08]

I disagree with the recently merged process.exit(0) solution to this issue.

The application making use of inquirer may wish to do unrelated cleanup via process.on('SIGINT', ...) before exiting normally. Conversely, the application may be running in a shell script where a normal exit after Ctrl+C would incorrectly execute subsequent commands in the script. https://www.cons.org/cracauer/sigint.html explains this much better than I could.

The correct way to handle this is process.kill(process.pid, 'SIGINT'). If the application has a listener handling this then it can deal with it how it wants, if it's not listening then the signal is correctly propagated to other processes.

[adamreisnz]

@welwood08 I can confirm that using process.kill(process.pid, 'SIGINT'); has the desired effect as well. I will create a PR for that. Thanks for pointing this out.

[b4dnewz]

I just tried with the version 6.x and my SIGINT handler never gets called unless I remove this line: process.kill(process.pid, 'SIGINT') from onForceClose function.. that is strange, is not supposed to forward the call to any listener? In my case it just kill the process, no SIGINT/exit handlers are called

[junfeisu]

I also have the same problem in the 6.x version.I found the problem is in this.close() method in ui/baseUI.js.More specific is the combination of

this.rl.pause()
this.rl.close().

I also look the pause and close method in node readline.js.But I can not find the reason.
I hope someone can help me to understand it.
Thanks!

[jbreeden-splunk]

EDIT: I guess this is more a comment on the issue that b4dnewz & junfeisu had, rather than the OP, in that I couldn't catch my SIGINT at all (I did not have to send it to my process twice). Still, I hope this is helpful for others that land here looking for signal handling issues

I thought I was hitting this issue with version 6.2, but it turned out not to be an issue with inquirer, it was more with the way signal handlers work in node.

Posting my solution here in case it's helpful for others:

My issue was, in short
That when inquirer encounters a SIGINT, it closes the underlying readline interface, which I believe cancels any handlers for readline input. If the only thing your app is doing is waiting for the inquirer prompt result, that means that the event loop will no longer be considered "alive", so the process will exit with a zero exit code.

You might think that your signal handler would be ref'ed by the event loop, as any socket listener or file read request is, but that is not the case. So even though you may have set a process.on("SIGINT", ...), the process will actually exit before it gets a chance to run your signal handler.

The solution was
To maintain a ref'ed handle in the event loop until I'm done listening for signals (or, equivalently in my case, until I'm done with my inquirer prompting).

My implementation of this solution

/**
 * This class is required for proper signal handling of
 * inquirer-based programs.
 *
 * USAGE:
 *
 *    async function myTask() {
 *      // Create a signal ref for the signal you'd like to catch
 *      const signalRef = new SignalRef(
 *        'SIGINT',
 *        () => { process.exit(1); }
 *      );
 *
 *      try {
 *        await doYourInquirerStuff();
 *      } finally {
 *        // Release the ref to avoid keeping the loop alive forever.
 *        signalRef.unref();
 *      }
 *    }
 *
 * NOTE: If you're not going to exit in your signal handler,
 * consider whether you want to catch the signal only once
 * (in which case maybe have it unref itself) or many times.
 *
 * See: https://github.com/SBoudrias/Inquirer.js/issues/293
 */

module.exports = class SignalRef {
    constructor(signal, handler) {
        this.signal = signal;
        this.handler = handler;

        process.on(this.signal, this.handler);

        // This will run a no-op function every 10 seconds.
        // This is to keep the event loop alive, since a
        // signal handler otherwise does _not_. This is the
        // fundamental difference between using `process.on`
        // directly and using a SignalRef instance.
        this.interval = setInterval(() => {}, 10000);
    }

    unref() {
        clearInterval(this.interval);
        process.removeListener(this.signal, this.handler);
    }
};

[justin-calleja]

@jbreeden-splunk thanks!

Is there a way to go back to the last prompt by inquirer after handling the signal? (ideally without bookkeeping from my end)

[SBoudrias]

Closing this ticket as stale. Open new issues if you encounter problem with the new Inquirer API.