Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Update documentation on readline (continuation) #4245

Closed
redchair123 opened this issue Nov 7, 2012 · 2 comments
Closed

Update documentation on readline (continuation) #4245

redchair123 opened this issue Nov 7, 2012 · 2 comments

Comments

@redchair123
Copy link

The documentation is still inconsistent. It says:

Emitted whenever the input stream receives a \n, usually received when the user hits enter, or return. This is a good hook to listen for user input.

Your comment in #4243 implicitly assumes that readline will also emit events when the output stream receives a '\n'. The docs should be updated to reflect this.

@TooTallNate
Copy link

Patch welcome.

@nchelluri
Copy link

Hi @TooTallNate and @Niggler, I think that this issue should still be open.

I'm not 100% sure of how the project structure works but my understanding is that the open issues in this v0.x-archive repo that are still valid for the current stable version of nodejs/node should stay open. You can see from my output below that this is, IMO, still a valid issue, so I think it should be reopened.

The issue was first described here: #4243 (comment) - the documentation should probably say something like Emitted whenever either the input stream **or the output stream** receives a \n. (I think in nodejs/node master there will be a bit more text because there was some copy changed around \r\n newlines, but the gist of this comment still applies.)

Please take a look at my repro cases and let me know if you agree. If so, I can put together a small PR for the docs [here (from the current nodejs/node master branch)](https://github.com/nodejs/node/blob/997a3be3af1f202dc8058ee1072cbd4b4b77895c/doc/api/readline.markdown#event-line - live, I believe, at https://nodejs.org/api/readline.html#readline_event_line).

1. If you have a line event handler that calls rl.write(strWithNewline), and any line event is fired, you will get a stack overflow. The workaround here is to write to the output stream directly rather than delegating this functionality to the readline instance, i.e. use rl.output.write(strWithNewline) instead (workaround from #4243 (comment)).

Demonstration in v5.9.0:

readline (master *#) $ cat writing-line-to-output-fires-line-event.js 
#!/usr/bin/env node

const readline = require('readline');
const rl = readline.createInterface(process.stdin, process.stdout);

rl.setPrompt('minishell:node-' + process.version + ' % ');
rl.prompt();

rl.on('line', (line) => {
  rl.write(line + '\n');
  rl.prompt();
}).on('close', () => {
  process.exit(0);
});
readline (master *#) $ ./writing-line-to-output-fires-line-event.js > output.txt 2>&1; cat output.txt
a
minishell:node-v5.9.0 % events.js:90
    handler.call(self, arg1);
            ^

RangeError: Maximum call stack size exceeded
    at emitOne (events.js:90:13)
    at Interface.emit (events.js:182:7)
    at Interface._onLine (readline.js:211:10)
    at Interface.<anonymous> (readline.js:341:12)
    at Array.forEach (native)
    at Interface._normalWrite (readline.js:340:11)
    at Interface.write (readline.js:310:49)
    at Interface.<anonymous> (/Users/nchelluri-user/dev/js/readline/writing-line-to-output-fires-line-event.js:10:6)
    at emitOne (events.js:90:13)
    at Interface.emit (events.js:182:7)
readline (master *#) $ 

2. To prove that the issue is not due to us writing to stdout via stdin or vice versa or some such weirdness which would logically cause some kind of infinite loop, I picked a completely different output stream and was able to reproduce the behavior: the rl.write(strWithNewline) function triggers line handlers, and if we call rl.write(someOtherStrWithNewline) itself in any of these handlers, we'll still get the stack overflow.

readline (master *#) $ cat writing-line-to-output-fires-line-event-even-if-output-stream-differs.js 
#!/usr/bin/env node

const readline = require('readline');
const fs       = require('fs');
const rl       = readline.createInterface(process.stdin, fs.createWriteStream('/tmp/still-overflows.txt'));

rl.setPrompt('type some stuff> ');
rl.prompt();

rl.on('line', (line) => {
  rl.write(line + '\n');
  rl.prompt();
}).on('close', () => {
  process.exit(0);
});

rl.write("about to cause a stack overflow!\n");
rl.close();
readline (master *#) $ ./writing-line-to-output-fires-line-event-even-if-output-stream-differs.js
readline.js:308
Interface.prototype.write = function(d, key) {
                                    ^

RangeError: Maximum call stack size exceeded
    at Interface.write (readline.js:308:37)
    at Interface.<anonymous> (/Users/nchelluri-user/dev/js/readline/writing-line-to-output-fires-line-event-even-if-output-stream-differs.js:11:6)
    at emitOne (events.js:90:13)
    at Interface.emit (events.js:182:7)
    at Interface._onLine (readline.js:211:10)
    at Interface.<anonymous> (readline.js:341:12)
    at Array.forEach (native)
    at Interface._normalWrite (readline.js:340:11)
    at Interface.write (readline.js:310:49)
    at Interface.<anonymous> (/Users/nchelluri-user/dev/js/readline/writing-line-to-output-fires-line-event-even-if-output-stream-differs.js:11:6)
readline (master *#) $ ll /tmp/still-overflows.txt 
-rw-r--r--  1 nchelluri-user  wheel  0 24 Mar 03:35 /tmp/still-overflows.txt
readline (master *#) $ 

3. You can see evidence of this still tripping people up in the nodejs/node repo: nodejs/node#4402 (comment)

Fiinally, to be as explicit as possible, I believe (like @Niggler in filing #4243) that the module is doing the wrong thing by firing the line event when a newline is written to the output stream. I don't think that writing to the output stream should ever fire this event; the module is called readline after all:

readline (master *#) $ man bash | grep -C1 'the library that handles reading input'
READLINE
       This  is  the library that handles reading input when using an interac-
       tive shell, unless the --noediting option is given at shell invocation.
readline (master *#) $ 

Hopefully this was not too much material on such a small subject...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants