jonaslu / ain

A HTTP API client for the terminal
MIT License
601 stars 13 forks source link

stderr is piped to stdout #17

Closed erikeah closed 7 months ago

erikeah commented 1 year ago

How to replicate error:

This ain file should output connection headers to stderr and print a json to stdout.

[Host]
http://localhost:3000/places

[Headers]
Content-Type: application/json

[Backend]
curl

[BackendOptions]
-sS -D /dev/stderr

But what actually happen is that stderr (the headers) are printed on stdout.

You can check this by running this line and reviewing the content of headers.txt file:

ain the_code_provided.ain 2>headers.txt

Why I consider that this should not be like this?

  1. Because as user I expect to be printed to stderr
  2. Because stderr could be used for logging and will allow to do this with the provided example :
    ain the_code_provided.ain | jq

    And don't make crash the jq parser.

Thanks in advanced!

jonaslu commented 1 year ago

Thanks for reporting! I'll have a think of the impact of changing the behaviour (which was a conscious decision to capture all output in the order the backend prints it): https://github.com/jonaslu/ain/blob/main/internal/pkg/call/curl.go#L89

In the meantime, you can work around this by doing:

Host]
http://localhost:3000/places

[Headers]
Content-Type: application/json

[Backend]
curl

[BackendOptions]
-sS -D /proc/${PID}/fd/2

PID=$$ ain the_code_provided.ain

That will print headers to the shells stderr and output to stdout

erikeah commented 1 year ago

Thanks a lot for quick response! Once you have decided what to do let me know and may I can submit PR.

jonaslu commented 1 year ago

Hi again!

I agree with this. Output from the backend can even intermingle, so separating them solves that too: https://unix.stackexchange.com/questions/476080/what-prevents-stdout-stderr-from-interleaving

For the delay: I'm in the middle of a major overhaul of the internals and wanted to see what impact it had, but it looks like doing this won't collide that much so go ahead and do the PR.

erikeah commented 1 year ago

Great! Then once I have the code, I will open PR!

erikeah commented 1 year ago

Hi Jonas, today I had a bit of time and I did the task as you can see in my fork, however I find that I have introduced a lot of boiler plate on the runAsCmd functions.

What do you think if I create a new function share for all backends to run the command and contain the boiler plate?

Pseudo-code based on my fork (internal/pkg/call/call.go):

func CallBackend(ctx context.Context, callData *data.Call, leaveTmpFile, printCommand bool) (Output, error) {
       ...

    if printCommand {
        if command, err := backend.getAsString(); err != nil {
            return Output{StdErr: []byte(command)}, err
        } else {
            return Output{StdOut: []byte(command)}, nil
        }
    }

    cmd, err := backend.GetCmd(backendTimeoutContext) // May returns the exec.CommandContext
        output, err := runCmd(cmd)

        ...
    return output, nil
}
jonaslu commented 1 year ago

Hard to tell, better if you do it as a commit on top of your fork and we can look at it then. There's always git reset.

erikeah commented 1 year ago

I will give a try this weekend then.

jonaslu commented 7 months ago

Fixed in commit eb9fa6c, closing this