Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stderr is piped to stdout #17

Closed
erikeah opened this issue Jun 28, 2023 · 8 comments
Closed

stderr is piped to stdout #17

erikeah opened this issue Jun 28, 2023 · 8 comments

Comments

@erikeah
Copy link

erikeah commented Jun 28, 2023

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
Copy link
Owner

jonaslu commented Jun 30, 2023

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
Copy link
Author

erikeah commented Jul 4, 2023

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

@jonaslu
Copy link
Owner

jonaslu commented Sep 22, 2023

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
Copy link
Author

erikeah commented Sep 24, 2023

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

@erikeah
Copy link
Author

erikeah commented Oct 4, 2023

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
Copy link
Owner

jonaslu commented Oct 5, 2023

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
Copy link
Author

erikeah commented Oct 6, 2023

I will give a try this weekend then.

@jonaslu
Copy link
Owner

jonaslu commented Mar 30, 2024

Fixed in commit eb9fa6c, closing this

@jonaslu jonaslu closed this as completed Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants