When `spago run`, if the target cli app manipulates console interface it may not terminate properly

[edwardw]

Just use the example from purescript-node-readline repo:

module Main where

import Prelude

import Effect (Effect)
import Effect.Console (log)
import Node.EventEmitter (on_)
import Node.ReadLine (close, createConsoleInterface, lineH, noCompletion, prompt, setPrompt)

main :: Effect Unit
main = do
  interface <- createConsoleInterface noCompletion
  setPrompt "(type 'quit' to stop)\n> " interface
  prompt interface
  interface # on_ lineH \s ->
    if s == "quit" then close interface
    else do
      log $ "You typed: " <> s
      prompt interface

If spago bundle then node index.js, after typing quit the app terminates. But if spago run directly, it hangs. It terminates only after pressing ctrl-d. With spago run --verbose, it can be deduced that the hang must have happened here: src/Spago/Cmd.purs#L59.

This happened with spago version 0.93.9. I also checkout out this repo and built a fresh version from head. It also happened there.

[edwardw]

Also dug a little bit around node documentation. Must be some reason that there exist three flavors of stream piping API stream.pipe, stream.pipeline and the experimental stream.compose. But better leave that to you node experts.

[JordanMartinez]

Hm... This is probably a bug with node-execa. Spago is calling subprocess.stdin.shareParentProcessStdin, so the parent stdin is being shared with the subprocess stdin.

[f-f]

We're not using the latest execa version - @JordanMartinez do you think that "just" upgrading to latest might fix this?

[JordanMartinez]

I'm not sure. I'll have to see what goes on in execa.

[JordanMartinez]

So, after I updated Test/Prelude.purs' spago' to use spawn rather than exec, I added the following test. Note: the node-readline code below is v8.1.0, not v7.0.0 in the opening thread.:

    Spec.itOnly "can run interactive programs" \{ spago, spago' } -> do
      spago [ "init" ] >>= shouldBeSuccess
      spago [ "install", "node-readline" ] >>= shouldBeSuccess
      FS.writeTextFile (Path.concat [ "src", "Main.purs" ]) $ Array.intercalate "\n"
        [ "module Main where"
        , ""
        , "import Prelude"
        , ""
        , "import Effect (Effect)"
        , "import Effect.Console (log)"
        , "import Node.EventEmitter (on_)"
        , "import Node.ReadLine (close, createConsoleInterface, closeH, lineH, noCompletion, prompt, setPrompt)"
        , ""
        , "main :: Effect Unit"
        , "main = do"
        , "  interface <- createConsoleInterface noCompletion"
        , "  setPrompt \"(type 'quit' to stop)\\n> \" interface"
        , "  prompt interface"
        , "  interface # on_ closeH do"
        , "    log \"interface was closed\""
        , "  interface # on_ lineH \\s -> do"
        , "    log $ \"You typed: \" <> show s"
        , "    if s == \"quit\" then do"
        , "      log $ \"calling rl.close()\""
        , "      close interface"
        , "      log $ \"called rl.close()\""
        , "    else do"
        , "      log $ \"Looping\""
        , "      prompt interface"
        ]
      spagoProcess <- spago' StdinNewPipe [ "run" ]
      let delay200Ms = Aff.delay $ Milliseconds 200.0
      delay200Ms
      spagoProcess.stdin.writeUtf8 "foo\n"
      delay200Ms
      spagoProcess.stdin.writeUtf8 "bar\n"
      delay200Ms
      spagoProcess.stdin.writeUtf8 "quit\n"
      delay200Ms
      spagoProcess.stdin.writeUtf8End "quit\n"
      delay200Ms
      spagoProcess.result >>= shouldBeSuccess

which outputs the following:

✅ Selecting package to build: 7368613235362d376c5766502b59557a4c3459473333656f2b

Downloading dependencies...
Building...
[1 of 2] Compiling Main
[2 of 2] Compiling Test.Main
           Src   Lib   All
Warnings     0     0     0
Errors       0     0     0

✅ Build succeeded.

(type 'quit' to stop)
> You typed: "foo"
Looping
(type 'quit' to stop)
> You typed: "bar"
Looping
(type 'quit' to stop)
> You typed: "quit"
calling rl.close()
interface was closed
called rl.close()
You typed: "quit"
calling rl.close()
called rl.close()
STDOUT:
(type 'quit' to stop)
> You typed: "foo"
Looping
(type 'quit' to stop)
> You typed: "bar"
Looping
(type 'quit' to stop)
> You typed: "quit"
calling rl.close()
interface was closed
called rl.close()
You typed: "quit"
calling rl.close()
called rl.close()

I noticed that the program hangs if I don't call stream.stdin.end (or the writeUf8End part above). So, maybe we need to call stream.stdin.end after detecting that CTRL+D was pressed?

[JordanMartinez]

After more investigation (and re-reading the opening thread), the issue here seems to be in node-readline. For whatever reason (and as shown above), calling close interface isn't actually closing the interface.

Also, from the Node docs:

Calling rl.close() does not immediately stop other events (including 'line') from being emitted by the readline.Interface instance.

[JordanMartinez]

This might be it. At the very bottom of readline.createInterface(options):

When creating a readline.Interface using stdin as input, the program will not terminate until it receives an EOF character. To exit without waiting for user input, call process.stdin.unref().

[f-f]

That makes sense, but I still don't get why it behaves differently when run directly with Node vs running it through Spago - it can't just be readline, it sounds like there is something that we are missing in how we set up the streams?

[JordanMartinez]

Yeah, that's what I'm trying to figure out myself...

[JordanMartinez]

There's seemingly a difference between

Terminal
 |
 +- node run.js

and

Terminal
 |
 +- node spago.js
    |
    +- node run.js

[JordanMartinez]

Regardless, the program written in the opening thread should not need to be changed in order for spago run to terminate once the first quit line is entered. If it works via bundling and node, then it should work via spago run.

So far, my experimentation has shown that the program only terminates if we end the child process' stdin. However, I don't see how spago could know when to end the child process stdin without knowing about the program being run itself.

AFAIK, the child process does not terminate because something is keeping the event loop alive, and that something appears to be the lack of an EOF sent to the child process' stdin in this unique situation where readline is using stdin as its input stream.

Maybe this changes if we run the child process directly via a shell? Otherwise, this might be one of those quirky Node bugs.

On a different note, I recall James Brock hitting something similar with node-streams. Something about stdin never actually emitting an end event. Unfortunately, I can't figure out where that was discussed.

[JordanMartinez]

I'd suggest using one of these three node libraries to see what's causing the child process to stay alive, but they're all written via CJS modules.

[f-f]

cc @jamesdbrock hopefully this is the same issue you had 😄