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 readline question results in unsettled promise on abortion #53497

Closed
029A-h opened this issue Jun 18, 2024 · 8 comments
Closed

Promise readline question results in unsettled promise on abortion #53497

029A-h opened this issue Jun 18, 2024 · 8 comments
Labels
promises Issues and PRs related to ECMAScript promises. readline Issues and PRs related to the built-in readline module.

Comments

@029A-h
Copy link

029A-h commented Jun 18, 2024

Version

v22.2.0

Platform

Linux LX-LAPII 6.9.3-arch1-1 #1 SMP PREEMPT_DYNAMIC Fri, 31 May 2024 15:14:45 +0000 x86_64 GNU/Linux

Subsystem

node:readline/promises

What steps will reproduce the bug?

It is not possible to properly handle a user's abortion of a promise readline question with SIGINT or Ctrl+D while rl.question waits for user input:

import * as readline from 'node:readline/promises';

const rl = readline.createInterface({
  input: process.stdin,
  output: process.stdout,
});

try {
  const answer = await rl.question('>>> ');
} catch {}

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

Every time when a prompt is aborted with Ctrl+C or Ctrl+D.

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

The expected behavior is to settle a promise on a readline close event, as shown in the following snippet:

import * as readline from 'node:readline';

const rl = readline.createInterface({
  input: process.stdin,
  output: process.stdout,
});

let rej = () => {};
rl.on('close', () => {
  console.log('closing');
  rej();
});

const answer = await new Promise((r, j) => {
  rej = j;
  rl.question('>>> ', r);
}).catch(() => console.error('interrupted'));

What do you see instead?

The promise remains unsettled, resulting in a warning that cannot be caught:

>>> Warning: Detected unsettled top-level await at file:///tmp/bug.mjs:9
  const answer = await rl.question('>>> ');

Additional information

This issue is opened based on the discussion in [stackoverflow] How to properly abort Node readline promise question?

@imanhpr
Copy link

imanhpr commented Jun 18, 2024

More Additional Information

According to the Nodejs documentation related to exit codes:

13 Unsettled Top-Level Await: await was used outside of a function in the top-level code, but the passed Promise never settled.

In the example below I expect to get 13 as an exit code in beforeExit and exit events but it doesn't.

import { createInterface } from "node:readline/promises";

process.on("exit", (code) => {
  console.log("\nexit:", code);
});

process.on("beforeExit", (code) => {
  console.log("\nbeforeExit:", code);
});

process.on("warning", (w) => {
  console.log("\nwar :", w);
});
const rl = createInterface({ input: process.stdin, output: process.stdout });

await rl.question("Enter something:");

Result :

Enter something:
beforeExit: 0

exit: 0
Warning: Detected unsettled top-level await at file:///home/imanhpr/Desktop/sandbox/node-box/sig.js:16
const data = await rl.question("Enter something:");

It's good to mention when I check the previous process exit code with echo $? it's 13 and it's fine.

@DanielVenable
Copy link
Contributor

I could try to work on this. Can someone assign me?

@avivkeller
Copy link
Member

Hi! If you have a solution, feel free to submit a PR!

@avivkeller avivkeller added readline Issues and PRs related to the built-in readline module. promises Issues and PRs related to ECMAScript promises. labels Jun 24, 2024
@jahudka
Copy link

jahudka commented Jan 23, 2025

Hi, just ran across this; funny thing is, rl.question() accepts an AbortSignal in the second argument, but when I tried installing a SIGINT handler and aborting the question manually, I got exactly the same result as before - meaning my signal handler wasn't even called. I hope I'm not overlooking something, but it looks as though the rl.question() method somehow hijacks the signal..?

@DanielVenable
Copy link
Contributor

@jahudka Can you share a code snippet?

@jahudka
Copy link

jahudka commented Jan 25, 2025

@DanielVenable sure thing, this is a minimal example which demonstrates what happens:

import { createInterface } from 'node:readline/promises';
import { stdin, stdout } from 'node:process';

const rl = createInterface({ input: stdin, output: stdout });
const ctrl = new AbortController();

process.on('SIGINT', function handleSignal() {
  console.log(`This should be printed when you press Ctrl+C, but it won't be`);
  process.off('SIGINT', handleSignal);
  ctrl.abort();
});

try {
  const response = await rl.question('Press Ctrl+C now!', { signal: ctrl.signal });
  console.log('This is never reached if you press Ctrl+C');
} catch {
  console.log('And neither is this');
}

console.log('Nor this');
rl.close();

shove that into a test.mjs file and run it with node test.mjs; after pressing Ctrl+C, only the "Warning: Detected unsettled top-level await" is printed and the script is immediately terminated

node -v: v23.3.0

@DanielVenable
Copy link
Contributor

@jahudka It is not rl.question but createInterface that hijacks the SIGINT signal. Once createInterface is called, the first SIGINT closes the readline interface. In your example that causes the process to exit because there is nothing left keeping it open. If there was something keeping it open, a second SIGINT would then call handleSignal. To make your example work as expected, replace process.on('SIGINT', function handleSignal() { with rl.on('SIGINT', function handleSignal() {.

@jahudka
Copy link

jahudka commented Jan 25, 2025

@DanielVenable Oh, wow, okay, makes sense. Never would've guessed that Readline would mess with signal handling like that.. But seeing the docs now and the fact that it handles SIGTSTP and SIGCONT it makes sense why it would do that. Still, I'd say it's somewhat unexpected and that it would be perhaps better to have those signal handlers be opt-in; or even just to have an option to opt out, which I don't see in the docs.

Anyway, I've just confirmed that if you install a SIGINT handler on the readline interface rather than on the process itself, you can abort the .question() call gracefully using an AbortController as shown in my example code and the warning will not be shown (although an AbortError will be thrown - but that can easily be caught).

So as far as I'm concerned, this could maybe do with a big fat warning in the docs, but otherwise I'd say it's working as intended. Thanks for helping me debug!

targos pushed a commit that referenced this issue Feb 2, 2025
Fixes: #53497
PR-URL: #54030
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
promises Issues and PRs related to ECMAScript promises. readline Issues and PRs related to the built-in readline module.
Projects
None yet
5 participants