readline.createInterface() is missing error handling for input stream

[fmcarvalho]

• Version: v12.13.1
• Platform: Windows 10 pro 64-bit
• Subsystem: readline

The iterator resulting from the readline.createInterface() should reject on the first iteration when the underlying input stream raises an error event. In fact, it is missing the error handling.
We can observe this problem regarding the example of processLineByLine() when we try to invoke the processLineByLine() for an absent file.
Even if we wrap the for await (const line of rl) {…} in a try/catch block we are unable to handle the error and the program finishes with:

events.js:187
      throw er; // Unhandled 'error' event
      ^

Error: ENOENT: no such file or directory, open 'input.txt'
Emitted 'error' event on ReadStream instance at:
    at internal/fs/streams.js:120:12
    at FSReqCallback.oncomplete (fs.js:146:23) {...}

A couple of workarounds has been discussed in the Stackoverflow question How to handle error from fs readline.Interface async iterator and jfriend00 proposes some alternatives to detour the problem: https://stackoverflow.com/a/59217504/1140754

[jfriend00]

To add a little more to this report, when using readline.createInterface() with files, it has no listener for the error event on the readStream it is using. This means it does not properly detect a file open failure on the passed in stream and does not detect any subsequent read errors on the stream (f the open succeeds). What happens is that the promise that the asyncIterator returns when using the for await (const line of rl) {…} syntax just never resolves or rejects when there's an error on the readstream.

Here's a simple way to reproduce the problem - code taken from readline doc:

const fs = require('fs');
const readline = require('readline');

async function processLineByLine(filename) {
  const fileStream = fs.createReadStream(filename);

  const rl = readline.createInterface({
    input: fileStream,
    crlfDelay: Infinity
  });
  for await (const line of rl) {
    // Each line in input.txt will be successively available here as `line`.
    console.log(`Line from file: ${line}`);
  }
}

processLineByLine("somefile.txt").then(() => {
    console.log("all done");
}).catch(err => {
    console.log(err);
});

And, just pass a filename that doesn't exist. If this is your entire program, you will notice that the process just exits without logging either the .then() or the .catch(). This is because the readstream gets an error internally (closing it), but the for await (const line of rl) is still waiting for its promise to resolve/reject. So, the nodejs process has nothing left to keep it open so it just exits. The same thing could happen even if the file open succeeded, but the readstream encountered an error while reading.

[rexagod]

Great catch! I've added error listeners in my PR above, and will fix the silent rejections in a later one.

Cheers!

[NunoSempere]

Here is a different variation on the same problem, which doesn't require the for await (const line of rl) {, but instead uses the rl.on(...) syntax. It surprised me that the rl.on('error') handler didn't catch the error.

const fs = require('fs');
const readline = require('readline');

async function processLineByLine(fileName) {
    const fileStream = fs.createReadStream(fileName);
        
    const rl = readline.createInterface({
      input: fileStream,
      crlfDelay: Infinity
    });

    rl.on('line', (line) => {
        throw new Error("Error message @ rl.on('line', ...) ") // This is never reached.
    })

    rl.on('close', () => {
        throw new Error("Error message @ rl.on('close', ...) ") // This is never reached. 
    });

    rl.on('error', (error) => {
        throw new Error("Error message @ rl.on('error', ...) ") // This is never reached (!).
    });
}

processLineByLine("nonExistentFile.txt").then(() => {
    console.log("All done")
}).catch(err => {
    console.log(err);
});

[jfriend00]

Here is a different variation on the same problem, which doesn't require the for await (const line of rl) {, but instead uses the rl.on(...) syntax. It surprised me that the rl.on('error') handler didn't catch the error.

It is true that the rl.on('error', ...) is not triggered in that example (which you can show by putting a console.log() in that event handler). And, that is a clear manifestation of a similar problem.

But the rest of the example is wrongly structured because you can't throw from a plain asynchronous event and expect the async function to see that exception and turn it into a rejected promise. In fact, because there are no await statements, this function just returns and resolves its promise immediately. The embedded throw statements never affect the promise returned by the async function and that has nothing to do with this bug. That's just a wrongly structured example.

[NunoSempere]

Checking fs.createReadStream and readline.createInterface with
console.log((fs.createReadStream).toString()) and console.log((readline.createInterface).toString()) gives us:

function createReadStream(path, options) {
  lazyLoadStreams();
  return new ReadStream(path, options);
}

and

function createInterface(input, output, completer, terminal) {
  return new Interface(input, output, completer, terminal);
}

respectively, so point taken, but it wasn't obvious that fs.createReadStream or readline.createInterface couldn't have thrown a (synchronous) error upon trying to validate an invalid input.