Method pause() on ReadLine should pause 'line' events

[jimlloyd]

This is a duplicate of #3628 which was closed without investigation. I believe the issue is valid and should be fixed, or the documentation should be changed.

See the last comment on #3628 for information on how to reproduce the bug. It's fine with me if you reopen #3628 and then close this issue as a duplicate.

FYI I'd like to be able to pause the input to a REPL, but fixing readline seems to be a prerequisite.

[amir-s]

You have used prompt at the end of your function onetime.

In the documentation they said:
http://nodejs.org/api/readline.html#readline_rl_prompt_preservecursor

This will also resume the input stream used with createInterface if it has been paused.

[jimlloyd]

Yes, I'm relying on the fact that prompt will resume the input stream. Note that rl.pause is called immediately after each 'line' event, and rl.prompt is only called after the line is entirely processed, even when the processing of the line happens asynchronously.

[amir-s]

Your current code doesn't process the line asynchronously. All of the processing and calling the prompt function happens one after each other.

Try to replace the line that you've called onetime() with setTimeout(onetime, 2000) and see the difference. The console will stop reading lines when line is processing.

Maybe I didn't get your point.

var readline = require('readline'),
rl = readline.createInterface(process.stdin, process.stdout);
rl.setPrompt('OHAI> ');
rl.prompt();
rl.on('line', function(line) {
    rl.pause();
    function onetime() {
        switch(line.trim()) {
            case 'hello':
                console.log('world!');
            break;
            default:
                console.log('Say what? I might have heard ' + line.trim() + '');
            break;
        }
        rl.prompt();
    }
    setTimeout(onetime, 2000);
}).on('close', function() {
    console.log('Have a great day!');
    process.exit(0);
});

[jimlloyd]

Please try it both ways, synchronously and asynchronously, by changing
which of the two lines is commented out. In both cases, run the code with
stdin from a file. For example, I have my script named test.js. I run it
like this:

./test.js < test.js

You'll see the output looks very different between the two runs. The sync
output looks like I expect. The async output is clearly wrong.

Jim

On Wed, Sep 10, 2014 at 12:27 PM, Amir Saboury notifications@github.com
wrote:

Your current code doesn't process the line asynchronously. All of the
processing and calling the prompt function happens one after each other.

Try to replace the line that you've called onetime() with setTimeout(onetime,
2000) and see the difference. The console will stop reading lines when
line is processing.

Maybe I didn't get your point yet.

var readline = require('readline'),
rl = readline.createInterface(process.stdin, process.stdout);
rl.setPrompt('OHAI> ');
rl.prompt();
rl.on('line', function(line) {
rl.pause();
function onetime() {
switch(line.trim()) {
case 'hello':
console.log('world!');
break;
default:
console.log('Say what? I might have heard ' + line.trim() + '');
break;
}
rl.prompt();
}
setTimeout(onetime, 2000);
}).on('close', function() {
console.log('Have a great day!');
process.exit(0);
});

—
Reply to this email directly or view it on GitHub
#8340 (comment).

[amir-s]

I ran both sync and async, the results was the same.
Can you please just put your output here? for both sync and async version.

[jimlloyd]

Please clone https://github.com/jimlloyd/node-readline-pause-bug and run make.
I have revised the script to more clearly demonstrate the bug. The Makefile will
run the script both sync and async, and the produce a side-by-side diff of the results.

[jimlloyd]

I configured this to run under Travis with both node v0.10 and v0.11. I intentionally constructed the Makefile such that Travis will declare a failed build, due to the bug.

See: https://travis-ci.org/jimlloyd/node-readline-pause-bug

The Travis job console output shows exactly what I see when I run locally on my MacBook.
See for example: https://travis-ci.org/jimlloyd/node-readline-pause-bug/jobs/35058518

[amir-s]

When you call pause on rl, it is going to stop emitting line event, but if you enter some lines when it is paused, they are going to be buffered on stdin. Then if you resume the rl (either calling resume on rl or just by calling prompt on it again), it is going to read from the buffer (which is already full of lines). That is a desired behaviour which exists in many programing languages.

[amir-s]

Do you expect that if you enter some lines when rl is paused, it should ignore those lines? and after resuming it it should wait for the user to enter a new line?

[jimlloyd]

Note that this is a 'line' event, not a 'data' event. The semantics should
be that lines are delivered one at a time, like streams with
{objectMode:true}. If the line event handler calls rl.pause(), then no more
line events should be delivered until rl.resume() or rl.prompt() is called.
Once rl.resume is called, only one line event should be emitted per tick.

It seems to me that the semantics of flowing-mode vs non-flowing-mode as in
ReadbleStream must somehow be reconciled in readline. Given that readline
currently only has events and pause/resume, it seems to be designed for
flowing mode only, so it is maybe not surprising that it has the current
behavior. Yet I argue that pause/resume are worthless for readline in node
applications. If input/output are TTY, then human reaction time is probably
never fast enough to matter. And given that readline is almost always used
in the context of a human with TTY input/output, then most application
developers using readline probably haven't even noticed the problem. But
why then is pause/resume even documented as existing? What are the uses
cases where application developers would want to use those features with
their current behavior?

I think you either need to make them work as I expect them to, or you need
to remove them as documented features.

On Fri, Sep 12, 2014 at 1:03 PM, Amir Saboury notifications@github.com
wrote:

Do you expect that if you enter some lines when rl is paused, it should
ignore those lines? and after resuming it it should wait for the user to
enter a new line?

—
Reply to this email directly or view it on GitHub
#8340 (comment).

[amir-s]

Well, it will probably make more sense to you when you look at those methods differently.
This may help: http://stackoverflow.com/questions/9812993/nodejs-stream-pausing-resuming-does-not-work-with-xmlhttprequest-but-works-with#comment18004634_9826405

Well, pause works but as you mention, it's advisory, and asynchronous. A very useful use-case of pause is within a proxy: you read from upstream, and write to the client. If the write to the client returns false, it means that the buffer is full, and you can send a pause to the backend. You will probably receive a few extra chunks of data from the backend, but not all the file. Without the pause, proxying a 1 GB file causes the whole file to be buffered into memory (and your Node.js process to explode). I hope this helps! – jpetazzo Nov 4 '12 at 15:46

[jimlloyd]

But see Mikeal's comment
#2989 (comment) in the
referenced Documentation Pull Request
#2989. He said that "in 0.9 it will
not be advisory, it will buffer data." This seems to me that he
acknowledged that the behavior should be as I expect.

On Fri, Sep 12, 2014 at 10:23 PM, Amir Saboury notifications@github.com
wrote:

Well, it will probably make sense to you when you look it those methods
differently.
This maybe helps:
http://stackoverflow.com/questions/9812993/nodejs-stream-pausing-resuming-does-not-work-with-xmlhttprequest-but-works-with#comment18004634_9826405

Well, pause works but as you mention, it's advisory, and asynchronous. A
very useful use-case of pause is within a proxy: you read from upstream,
and write to the client. If the write to the client returns false, it means
that the buffer is full, and you can send a pause to the backend. You will
probably receive a few extra chunks of data from the backend, but not all
the file. Without the pause, proxying a 1 GB file causes the whole file to
be buffered into memory (and your Node.js process to explode). I hope this
helps! – jpetazzo Nov 4 '12 at 15:46

—
Reply to this email directly or view it on GitHub
#8340 (comment).

[jasnell]

@jimlloyd ... is this still an issue for you?

[jimlloyd]

I've long since worked around this issue, so I have no personal urgency to see it be addressed. But I still believe that it truly is a flaw and in an ideal world resources would be allocated to fix it. Given that this is not an ideal world, I'll leave it up to you as to whether this issue (that supposedly was going to be fixed as far back as 0.9) will ever be fixed.

[jasnell]

@jimlloyd ... will at least mark it as a defer to the converged repo. That will at least keep it on the radar but we'll need someone to step up with a PR to make progress on it.

[jimlloyd]

+1

[silkentrance]

@jimlloyd what exactly was your work around?

[jimlloyd]

@silkentrace At the time I filed this issue, my workaround was to use repl-promise. But the full truth is that not long after filing the issue I abandoned the main use case (gremlin-repl) I had for a repl that can be deterministically driven from file input. As such, repl-promise never became a mature package. Let me know if you evaluate repl-promise and decide that it or something like it would serve your needs, as I might be willing to invest some more time into it.

[ACV2]

problem is easily replicated here...
/CODE/

var lineReader = require('readline').createInterface({
  terminal: false,
  input: require('fs').createReadStream('data.json')
});

lineReader.on('line', function (data) {
    lineReader.pause(); 
    syncP(data);
    
});


lineReader.on('pause', function (data) { 
    console.log('Waiting for event to finish:');
});

function syncP(data) {
    
    setTimeout(function() {
      console.log('Line from file:', data);
      lineReader.resume();
    },rand(100,3000));
}


function rand(min, max) {
  var argc = arguments.length;
  if (argc === 0) {
    min = 0;
    max = 2147483647;
  } else if (argc === 1) {
    throw new Error('Warning: rand() expects exactly 2 parameters, 1 given');
  }
  return Math.floor(Math.random() * (max - min + 1)) + min;

} 0-1; 

/CODE/

data file:
data.json

{"id":0,"name":"line 0","value":489}
{"id":1,"name":"line 1","value":148}
{"id":2,"name":"line 2","value":798}
{"id":3,"name":"line 3","value":766}
{"id":4,"name":"line 4","value":70}
{"id":5,"name":"line 5","value":825}
{"id":6,"name":"line 6","value":353}
{"id":7,"name":"line 7","value":175}
{"id":8,"name":"line 8","value":12}

you will see clearly the output should be 1,2,3,4,5,6... which is not the case so the semantic and the process ARE wrong.. (maybe is not meant for this kind of processing but still would be a really usefull fix)

[sterpe]

This is still jacked up in node 8.5.0.

Waiting for event to finish:
Line from file: {"id":5,"name":"line 5","value":825}
Waiting for event to finish:
Line from file: {"id":6,"name":"line 6","value":353}
Line from file: {"id":1,"name":"line 1","value":148}
Line from file: {"id":7,"name":"line 7","value":175}
Line from file: {"id":8,"name":"line 8","value":12}
Line from file: {"id":3,"name":"line 3","value":766}
Line from file: {"id":2,"name":"line 2","value":798}
Line from file: {"id":0,"name":"line 0","value":489}
Line from file: {"id":4,"name":"line 4","value":70}

I agree with @jimlloyd and @ACV2 -- I was thinking about using node's Readline to build some kind of ed-like line editor, but the inability to lock input briefly to complete any async processing makes the implementation absolutely useless.