Phantomjs: Create a non-blocking `system.stdin` implementation

[JamesMGreene]

_[email protected] commented:_

A person by the GitHub username of @burlito brought up that he/she would like to have a non-blocking stdin implementation available, which I agree makes sense.

@burlito's comment:
https://github.com/ariya/phantomjs/commit/f6c87221a775bb6dc3d5102951b4f333d714af8b#commitcomment-2423252

A non-blocking/asynchronous stdin should probably be the default but we can consider providing both implementations if there is value (more than just convenience) in maintaining the blocking/synchronous version.

This would also align with how Node.js stdin works as well: http://nodejs.org/api/process.html#process_process_stdin

Some implementation notes:

• http://stackoverflow.com/questions/1271784/using-qtextstream-to-read-stdin-in-a-non-blocking-fashion
• http://stackoverflow.com/questions/6878507/using-qsocketnotifier-to-select-on-a-char-device/7389622#7389622
• http://stackoverflow.com/questions/9741259/reading-asynchronously-from-stdin-in-qt (provides alternative but likely less good implementation ideas)

Also should include some example scripts and, if possible, tests.

Thanks!

Disclaimer:
This issue was migrated on 2013-03-15 from the project's former issue tracker on Google Code, Issue #980.
:star2:   4 people had starred this issue at the time of migration.

[JamesMGreene]

_[email protected] commented:_

Also a note that implementing this successfully could give us the opportunity to make some nice higher-level stdin abstractions like TJ Holowaychuk's "commander" module in Node.js. It provides a very nice higher level, asynchronous abstraction over stdin, including entering masked passwords, choosing an option from a list, etc.:
http://visionmedia.github.com/commander.js/

I would love to see PhantomJS gain a similarly slick async module for common stdin requirements.

 
_Metadata Updates_

• Owner updated: ---

[JamesMGreene]

_[email protected] commented:_

It would be nice to update this for the 1.9 release so that we don't have to maintain backward compatibility for the stdin API (since it was added post-1.8).

 
_Metadata Updates_

• CC(s) added:
  
  
  
  • [email protected]
  
  
  • [email protected]
  
  
• Milestone updated: Release1.9 (was: FutureRelease)

[JamesMGreene]

_[email protected] commented:_

Note: this would be modifying or extending the implementation of system.stdin that was introduced to resolve Issue #10333.

Update: Fixed issue number, per @execjosh's note below.

[execjosh]

@JamesMGreene: Your imported comment links to an unrelated issue ("Allow to specify ssl protocol"). I think it should have been translated to #10333.

[execjosh]

Should we strive for node compatibility (allowing code to be shared cross-platform)? I was initially thinking that we should probably align the fs methods with node (e.g., read and readSync). However, this will almost certainly break any implementations that depend on the current _synchronous_ implementation. If we want to go that route, then I think that we should hold off until the next major release (v2.0) to make the switch.

For now, I'm thinking of just adding an optional parameter, callback, to the methods, which can then switch to _async_ mode when typeof callback === "function". This should allow a partial compatibility with node.

I'm open to other suggestions, too, though!

[JamesMGreene]

@exejosh: Yeah. In general, PhantomJS has obviously centralized around making synchronous methods the default and then adding *Async variants afterward. Whether or not this is the right choice is a topic for a bigger discussion, so I'll start something on the mailing list.

As for this particular issue and the fact that the stdin API is now released, I'd say that my request should be switched (unless a discussion suggests otherwise) to leave stdin.read as it is now and instead add a stdin.readAsync.

[JamesMGreene]

Related discussion link: https://groups.google.com/d/topic/phantomjs/6l_0cFXSBJg/discussion

[execjosh]

I've got a working POC in a new branch in my fork. Check it out! Here's an example script:

# async-read.coffee

fs = require "fs"
system = require "system"

system.stdin.read 12, (err, data) ->
  console.log "system.stdin:", err, data
  return

console.log "Hello"

fs.read "Makefile", (err, data) ->
  console.log "Makefile:", err, data.substring 0, 12
  return

console.log "There"

fs.open "third-party.txt", "r", (err, f) ->
  if err?
    console.log "third-party.txt:", err
    return
  f.read (err, data) ->
    f.close()
    console.log "third-party.txt:", err, data.substring 0, 12
    return
  return

console.log "!"

Run from the phantomjs root and typing "abcdefghijklmnopqrstuvwxyz[ENTER]" on the terminal:

$ ./bin/phantomjs async-read.coffee
Hello
There
!
Makefile: null ############
third-party.txt: null This documen
abcdefghijklmnopqrstuvwxyz
system.stdin: null abcdefghijkl
^C

[JamesMGreene]

@execjosh: Awesome, thanks for the speedy turnaround! :)

Feedback:

1. While I appreciate the cleverness of switching between sync and async based on the arguments provided, we cannot easily do the same for most of our other [sync vs. async] methods due to a variable number of arguments. For ease of maintenance and to avoid consumer confusion, I'm thinking we are probably better off sticking with the API design common in the rest of PhantomJS: read will read synchronously and a second method, readAsync, will read asynchronously. Could you update your branch for that?
2. Also, I'm thinking that it would also make sense for readLine to have an asynchronous equivalent as well, i.e. readLineAsync. Would you agree with that, or am I missing something?

And yes, I know you said the branch is only a work-in-progress, so perhaps my feedback is just jumping the gun! If so, sorry in advance. :)

[execjosh]

@JamesMGreene: And thanks for having a look and giving constructive feedback!

Ah, I understand your points and agree with them. Adding the *Async methods is much less intrusive! I've updated the branch; so, I hope you can have another look. I've also put async-read.coffee into the examples folder for convenience.

BTW, I've only thought about reading and opening Files, and haven't done anything for writing yet (i.e., writeAsync)...

[zanona]

bump, guys please consider adding this feature, it seems like a good deal that could be really useful

For now I am doing like this:

I am not sure if this is the right way to do it or even if it is great performance-wise, however it worked fine with the exception that a first STDIN request must be made for the program so it initialises.

test.js

console.log('initialising program, waiting for user input');

var stdin = require('fs').open('/dev/stdin', 'r');

setInterval(function () {
    var line = stdin.readLine();
    if (line) { console.log('>>', line); }
}, 100);

console.log('logic running async');

then you can create a FIFO pipe and attach it to the process

$ mkfifo PIPE
$ phantomjs test.js < PIPE

and from there on another terminal window you could:

echo hello world > PIPE

the ouput would be:

initialising program
logic running async
>> hello world

I don't think this is going to work on windows machines. and again, this might be really bad performance-wise since it uses a defined interval to catch user's input, but if I want to debug a program it is a quick and easy way to do so, by evaluate the input from there.

[ghost]

Would love to be able to retrieve stdin to phantom via an event listerner!

[JamesMGreene]

@zanona @deshawnbw: Features will only be added in minor releases/versions (but not in point/patch releases/versions), which means that this feature will not make it in till _at least_ PhantomJS 2.0.

[Glench]

Is there any word on this?

[seosgithub]

I have an asynchronous implementation of stdin in my project. It relies on the server and multiplexing the $stdin into a socket. The setInterval is used as any exceptions thrown on the server's context of execution do not call onError, so I defer the exceptions into a queue which _does_ execute on a context that does properly calls onError.

https://github.com/sotownsend/BooJS/blob/6a8ec85d1979a8c5a4c2e38ad3fb6ba2cb43b6c8/lib/boojs.rb

[zackw]

This seems closely related to #13970 - but that's about the child_process API, whereas if I understand correctly this is about phantomjs's own stdin?

[ghost]

How far are you with solution to this problem? As everyone here mentioned, there should be any solution asap, because it makes a lot problems not only with communication, but even events listeners.

[stale[bot]]

Due to our very limited maintenance capacity (see #14541 for more details), we need to prioritize our development focus on other tasks. Therefore, this issue will be automatically closed. In the future, if we see the need to attend to this issue again, then it will be reopened. Thank you for your contribution!