Skip to content

readline: line emitted after close while processing text file #22615

Description

@lych77
  • Version: v10.9.0
  • Platform: Windows 7 64-bit

Test code:

'use strict';

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

let sampleText = '';
for (let i = 0; i < 10; i++) {
	sampleText += `This is line ${i}\n`;
}
fs.writeFileSync('sample.txt', sampleText);

const file = fs.createReadStream('sample.txt', {encoding: 'utf-8'});
const reader = readline.createInterface(file);

reader.on('line', ln => {
	console.log(ln);
	if (ln.endsWith('4')) {
		reader.close();
	}
});

reader.on('close', () => {
	console.log('closed');
});

Output:

This is line 0
This is line 1
This is line 2
This is line 3
This is line 4
closed
This is line 5
This is line 6
This is line 7
This is line 8
This is line 9

Should this be supposed to happen? What should I do if I want to cancel the reading process under some condition? Is it the only way to maintain some flag myself and check it in the 'line' handler and just ignore the overheads?

Activity

  1. cjihrig commented on Sep 2, 2018

    @cjihrig
    Contributor

    The issue here is that the readable stream (file) emits the entire file in a single 'data' event. Each 'data' event queues up 0 or more 'line' events. So, by the time close() relinquishes control over the stream, all of the 'line' events are already queued up.

    A few things that could be done:

    • track state yourself (as mentioned by OP)
    • Set a low value of highWaterMark for file.
    • Monkey patch Interface.prototype._onLine() (probably shouldn't do that though).
    • Core could do something, but this seems better off in userland code.
  2. lych77 commented on Sep 3, 2018

    @lych77
    Author

    Thanks, I've got the picture. Yes there are workarouds, I can also remove the 'line' listener as well when closing the interface. I might just be interested in if this is an intended behavior, or is considered as a problem too by the official team (no matter how late to fix it, as it's so trivial).

  3. cjihrig commented on Sep 3, 2018

    @cjihrig
    Contributor

    IMO, this is intended behavior. There is already a note in the pause() docs that hints at it. I've opened #22679 to add a similar warning to close().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions