mage icon indicating copy to clipboard operation
mage copied to clipboard

Collect both stdout and stderr in mage/sh

Open coryrc opened this issue 4 years ago • 8 comments

sh.BothOutput and sh.BothOutputWith

follows existing naming conventions.

coryrc avatar Jun 23 '21 23:06 coryrc

@davidgamba (sorry, I can't find the comment in the UI, only e-mail!?)

CombinedOutput returns stdout and stderr in one stream, here we're returning them as separate streams. There isn't precedent in either os/exec or this package so I thought "BothOutput" but not wedded to it, of course.

coryrc avatar Jun 24 '21 04:06 coryrc

@coryrc I sent that message before reading the code more in depth and that is why I deleted it afterwards, I realized it made no sense since the use case is different.

DavidGamba avatar Jun 24 '21 15:06 DavidGamba

@DavidGamba Thought I was losing it!

Hey, since you're here, what do you think of *Output*V() (added V) versions which simultaneously stream to os.Stdout and os.Stderr? Or are we just encoding all permutations of exec.Cmd in variable names?

coryrc avatar Jun 24 '21 15:06 coryrc

Adding a *V version to BothOutput doesn't seem necessary since you are capturing the output, you could print it right after (though it won't be "live").

What is your use case here? I haven't found many cases where capturing stderr out of context within stdout is what I want and I would love to know how you plan to use this.

DavidGamba avatar Jun 24 '21 15:06 DavidGamba

As part of CI tests I can popup errors to the user why it stopped. Stdout can have too much context and just Stderr is often enough to figure out what went wrong without having to page through a lot of output.

Without live streaming, you don't get any error message if the process is terminated due to timeout, which makes diagnosing hangs a real pain. Also an annoying user experience running locally, because it's hard to know if something is hung or slow.

coryrc avatar Jun 24 '21 18:06 coryrc

@coryrc any further thoughts on the issues you've identified?

jufemaiz avatar Nov 03 '22 07:11 jufemaiz

@coryrc any further thoughts on the issues you've identified?

No. I just used said functions in my own library, it worked fine. I haven't used it for a while though.

coryrc avatar Nov 04 '22 17:11 coryrc

Aaaah fair enough.

jufemaiz avatar Nov 25 '22 00:11 jufemaiz