process :inherit semantics

[ikappaki]

When any of the babashka.process/process :in, :out and :err OPTS is set to :inherit, their stream is inherited from the parent process' corresponding stream, which, for clarification, is not the same as the parent process' Clojure *in*, *out* and *err* "streams".

This at first sight might cause confusion to clojurians, because they are accustomed to think at the clojure context and thus their first expectation is for the clojure streams to be inherited. It can also cause some confusing at first puzzling effects, such as the error output of a failing babashka.process/shell call during an nREPL session to be printed by the nREPL server than send over to the REPL client (since the parent java process in this case is the nREPL server), i.e. the error message is likely to be "hidden" from the user.

Was this redirection at the java level a design decision or rather happened for convenience, or maybe both? I'm trying to ascertain whether it would be a more convenient option to inherit from the clojure streams by default or add an additional option for this or just update the documentation to elaborate just a bit more on the inheritance so that clojurians are less likely to be confused by the side effects. What do you think? :)

Looking at the code, the redirection to the java process happens in the build fn which works at the java ProcessBuilder level:

process/src/babashka/process.cljc

Line 221 in bd203b7

(defn- build

(defn- build
  (^java.lang.ProcessBuilder [cmd] (build cmd nil))
  (^java.lang.ProcessBuilder [^java.util.List cmd opts]
   (let [
;; ...
         pb (cond-> (java.lang.ProcessBuilder. ^java.util.List cmd)
              dir (.directory (io/file dir))
              env (set-env env)
              extra-env (add-env extra-env))]
     (case out
       :inherit (.redirectOutput pb ProcessBuilder$Redirect/INHERIT)
       :write (.redirectOutput pb (ProcessBuilder$Redirect/to (io/file out-file)))
       :append (.redirectOutput pb (ProcessBuilder$Redirect/appendTo (io/file out-file)))
       nil)
     (case err
       :inherit (.redirectError pb ProcessBuilder$Redirect/INHERIT)
       :write (.redirectError pb (ProcessBuilder$Redirect/to (io/file err-file)))
       :append (.redirectError pb (ProcessBuilder$Redirect/appendTo (io/file err-file)))
       nil)
     (case in
       :inherit (.redirectInput pb ProcessBuilder$Redirect/INHERIT)
       nil)
     pb)))

To reproduce the Hudini effect with babashka.process/shell and Emacs:

1. Jack-in to babashka with Emacs Cider
2. In the repl run (babashka.process/shell "ls" "-----"), an error is thrown but there is no indication what has gone wrong, just that the process exited with error code 2:

clojure.lang.ExceptionInfo: 
{:type :sci/error, :line 1, :column 1, :message "", 
#...
}
 at sci.impl.utils$rethrow_with_location_of_node.invokeStatic (utils.cljc:128)
#...
Caused by: clojure.lang.ExceptionInfo: 
{:proc #object[java.lang.ProcessImpl 0x1d310663 "Process[pid=28332, exitValue=2]"], :exit 2
# ...

3. The error is hidden instead in the *nrepl-server ...* buffer:

Started nREPL server at 127.0.0.1:1667

For more info visit: https://book.babashka.org/#_nrepl

ls: unknown option -- ---
Try '/usr/bin/ls --help' for more information.

4. While if we nilify the :err OPTS we get some sense out of babashka.process/check with (babashka.process/shell {:err nil } "ls" "-----"):

clojure.lang.ExceptionInfo: ls: unknown option -- ---
Try '/usr/bin/ls --help' for more information.
;; ...
{:type :sci/error, :line 1, :column 1, 
Caused by: clojure.lang.ExceptionInfo: ls: unknown option -- ---
Try '/usr/bin/ls --help' for more information.

{:proc #object[java.lang.ProcessImpl 0x3b4ce6c0 "Process[pid=22280, exitValue=2]"],
;; ...

Thanks

[borkdude]

You can write to *out* and *err* using:

(process [...] {:out *out* :err *err*})

This is not the default because there is buffering overhead when you write to those.

The defaults are chosen like they are now so you can compose process:

$ bb -e "(require '[babashka.process :refer [process]]) (-> (process \"ls\") (process \"cat\" {:out :string}) deref :out)"

There are always trade-offs and no one default will make everybody happy.

[ikappaki]

Hi @borkdude,

Are you happy to include a reference or two about the Clojure standard streams*out* etc in the process docstring? :) There is no problem with the defaults, but rather setting the reader's expectations. For example, it took me a while to figure out what was happening (the expectation was that I should see an error in the nREPL client - but there was nothing neither in the REPL nor in the exception) and I didn't even think using {:err *err*} even after reading the docstrings and went through the debugging effort.

Just for the record, your suggestion works as expected:

user> (babashka.process/shell {:err *err*} "ls" "-----")
ls: unknown option -- ---
Try '/usr/bin/ls --help' for more information.
Execution error (IllegalArgumentException) at babashka.process/check (process.cljc:106).
Cannot open <#object[java.io.PrintWriter 0x2e52d652 "java.io.PrintWriter@2e52d652"]> as an InputStream.

Thanks

[borkdude]

Yes, I'd be happy to include this in the docstring. Do you want to submit a PR?

[ikappaki]

Yes, I'd be happy to include this in the docstring. Do you want to submit a PR?

#89