I have been using golang to automate some deploy processes and I had to use exec package to call some bash scripts.
I used exec.Command("/home/rodrigo/my-deploy.sh").CombinedOutput() and I saw his implementation
func (c *Cmd) CombinedOutput() ([]byte, error) {
if c.Stdout != nil {
return nil, errors.New("exec: Stdout already set")
}
if c.Stderr != nil {
return nil, errors.New("exec: Stderr already set")
}
var b bytes.Buffer
c.Stdout = &b
c.Stderr = &b
err := c.Run()
return b.Bytes(), err
}
I realized you can't assign c.Stdout when using CombinedOutput() and I think that's ok but the way it is informed to the api caller is not correct.
CombinedOutput() return an error when you are using it in a bad way, so if you are going to use CombinedOutput() then you shouldn't assign c.Stderr or c.Stdout previously, if you do that then you are going to receive an error.
But this error is not because your script throw an error, it is because you are using the api wrong, in that case I believe you should get a panic because a bad api usage should not being handled (I think).
I come from Java World and when you are using some method in the wrong way then you receive an RuntimeException, for example.
public void run(Job job) throws NotCompletedJob {
if (job.getId() != null) {
throw new IllegalArgumentException("This job should not have id");
}
job.setId(calculateId());
job.run();
}
With this signature I can know I'm wrong calling run(obj); with a Job that has an id and in fact I can distinguish if there is an error with my script or I'm using api in a wrong way.
NotCompletedJob is a checked exception so I must handle it but IllegalArgumentException is not so I could get it anytime. Catching IllegalArgumentException or any other RuntimeException is not always considered a good practice because they are indicating you have an error from programmers's point of view and it is not a possible expected error like NotCompletedJob.
Having said that, How can I differentiate between a programming error (bad api usages for example) from an expected error (script doesn't finished ok) with current CombinedOutput() implementation ?
To clarify my concern, I'm not saying that is wrong the current implementation of CombinedOuput, but I don't understand how the caller could distinguish if it is an error of the command being executed or an error caused from his bad api usage.
I believe that a best approach would be to panicking when the caller is using the api in a wrong way, as the same case when the caller is passing a nil reference to a function that expect a non nil reference (in fact this is the current behaviour).
I come from Java World and when you are using some method in the wrong way then you receive an RuntimeException.
You are in the Go world now. Therefore, that argument is invalid. Abandon Java.
The Go Programming Language Specification
Two built-in functions, panic and recover, assist in reporting and handling run-time panics and program-defined error conditions.
func panic(interface{}) func recover() interface{}While executing a function F, an explicit call to panic or a run-time panic terminates the execution of F. Any functions deferred by F are then executed as usual. Next, any deferred functions run by F's caller are run, and so on up to any deferred by the top-level function in the executing goroutine. At that point, the program is terminated and the error condition is reported, including the value of the argument to panic. This termination sequence is called panicking.
The Go Blog
The convention in the Go libraries is that even when a package uses panic internally, its external API still presents explicit error return values.
This page collects common comments made during reviews of Go code, so that a single detailed explanation can be referred to by shorthands. This is a laundry list of common mistakes, not a style guide.
See https://golang.org/doc/effective_go.html#errors. Don't use panic for normal error handling. Use error and multiple return values.
The usual way to report an error to a caller is to return an error as an extra return value.
Your Go server program is concurrently handling 100,000 clients. If an error occurs, report and handle it; always check for errors. DON'T crash all 100,000 clients with a panic. Go packages should not panic.
Read the Go documentation and the Go standard library code.