Idiomatic way to handle logic error vs programming error in golang

Question

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).

Maybe that should have been a `panic`, but it's a moot point, since the API cannot be changed in go1. — JimB, Jun 08 '18 at 14:27
@JimB Exactly but I believe It doen't matter if this can or not be changed, but it must have been a panic or an error. — rvillablanca, Jun 08 '18 at 14:33
What are you asking then exactly? Any tests would indicate incorrect usage, because it would always error. — JimB, Jun 08 '18 at 14:37
Because the function will always return the error when incorrectly used and the error text describes the problem well enough for the programmer to determine that it's a logic error, there's no need to programmatically distinguish logic errors from other errors in this scenario. — Charlie Tumahai, Jun 08 '18 at 14:39
@JimB I'm asking what is the correct way in Golang to differentiate an expected error from and programming error from caller's point of view. The example shows an example of an incorrect api usage. — rvillablanca, Jun 08 '18 at 14:41
@rvillablanca Maybe your Java code can be converted [to this](https://gist.github.com/alireza-ahmadi/29422b93115b16265f78789dd071d2ce) in Golang (and yes, it's a common pattern as far as I know) — Alireza Ahmadi, Jun 08 '18 at 14:42
Many packages provide helpers to let you check what type of error it is programmatically. While `os/exec` does not, you could still match against the string value of the error. — Adrian, Jun 08 '18 at 14:46
@Adrian of course I can, but this is not my question. I want to know if implementation of CombinedOutput() is the best way or not. I should be a panic or still being an error — rvillablanca, Jun 08 '18 at 14:47
Asking "if the implementation of CombinedOutput() is the best way or not?" isn't useful here, since it's not for us to change. The API is set, so you work with it by either having a test to verify that you're using it correctly, or by matching the error string. — JimB, Jun 08 '18 at 15:05
If your question is about how your own functions should expose errors, post a question about that, rather than about a stdlib function that isn't up for debate. — Adrian, Jun 08 '18 at 15:13
The distinction between exception types in Java is part of the overall design for error handling Java. Go has a different way of handling errors. There's no need to programmatically distinguish logic errors from expected errors in Go. — Charlie Tumahai, Jun 08 '18 at 16:11

peterSO · Accepted Answer · 2018-06-08T15:52:48.033

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

Handling panics

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

Defer, Panic, and Recover

The convention in the Go libraries is that even when a package uses panic internally, its external API still presents explicit error return values.

Go Code Review Comments

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.

Don't Panic

See https://golang.org/doc/effective_go.html#errors. Don't use panic for normal error handling. Use error and multiple return values.

Effective Go

Errors

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.

Idiomatic way to handle logic error vs programming error in golang

1 Answers1