6

I cannot find a good confirmation if returning both non-nil value and non-nil error at the same time is conventional or not. Example:

func makeHTTPCall(...) string, error {
    resp, err := http.Post(...)
    if err != nil {
        return "", err
    }

    var r string
    if resp.StatusCode == http.StatusOK {
        if err := json.NewDecoder(resp.Body).Decode(&r); err != nil {
            return "", err
        }
        if err := resp.Body.Close(); err != nil {
            // should I return error here even if I have successful result?
            // or should I return both non-nil value and non-nil error?
            // or should i just log the error?
        }
        return r, nil
    }
}

Definitely it is allowed by compiler. Why I'm asking is because of the convention to check for error first. If I return both non-nil value and error client code will do the check for error first and will go on error handling path:

str, err := makeHTTPCall(...)
if err != nil {
   throw err
}

Is there any agreement around this?

Improve this question
4
  • 2
    I don't understand. The convention is to check the error first, it doesn't matter what the first value is if you're not going to use it. If the function has 2 returns values, you always need to return 2 values. Commented Jun 15, 2020 at 23:50
  • 2
    If err is non-nil, the other return value should be treated as undefined unless very well documented. Even if documented, I think that's a code smell -- a sign that you need to break up the function into smaller pieces whose errors can be handled separately. Commented Jun 15, 2020 at 23:54
  • Is this convention explicitly documented anywhere? I agree that it seems like Either monad which cannot have both right and left values. But there are exclusions from this rule in stdlib (see answer stackoverflow.com/a/62399082/2602998). Commented Jun 16, 2020 at 3:48
  • For the standard library function burak has mentioned, it returns a specific error (io.EOF) in the case where the other return is still usable. If it were some other (undocumented) error then it would not be possible to assume that any of the other return values are usable. Commented Jun 16, 2020 at 5:00

3 Answers 3

Reset to default
11

Although the general pattern for this is that if you have non-nil error the return value is undefined, there are well-known examples on the contrary, for instance, Reader.Read, which returns both a valid number and io.EOF in case of an EOF.

On the other extreme, gRPC libraries will fail if you both return a non-nil value and error at the same time.

So, this is to be decided case-by-case.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

What would you suggest in this case: return error or return value?
That's up to you. Depends a bit on the server-side of that call as well. Clearly server did what it is supposed to do. Is it going to roll it back if an error happens after the response is written? Will it even know?
gRPC libraries will fail if you both return a non-nil value and error at the same time - could you please point me to example docs/code?
Although the general pattern for this is that if you have non-nil error the return value is undefined: Is this a documented rule, an idiomatic convention, a social norm, something else?
It is not a rule, nor is it idiomatic, standard library io.Read for instance does not follow this convention. There are many others that follow it.
5

In general, it is pretty standard way to return errors: if a non-nil error is returned, all other values should be either nil, or zero, depending on the type. There is though one important exception.

In this blog entry https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully, Dave Cheney, a prominent go developer, makes an in-depth introduction of the Sentinel Errors concept.

Some errors are meant to signal that processing cannot continue due to a problem with the current state.

Dave Cheney writes:

«The name descends from the practice in computer programming of using a specific value to signify that no further processing is possible. So to with Go, we use specific values to signify an error.»

The most well-known sentinel error is already mentioned in other answers io.EOF. There are some other places in the standard go library, where sentinel errors are used, e.g. archive/zip.

There is a big section devoted to your question, and to sentinel errors, in Learning Go by Joe Bodner (pp.161—165), which I recommend in any case, not only for a detailed answer to your question.

Improve this answer

Comments

2

If you have decided that your function has completely failed at what it is trying to do, it is a good practice to return zero values for all non-error return values. This will avoid confusion or runtime errors (as much as possible) for anyone who chooses to call your function and forgets to check for errors. If your function has only produced junk results, it's better for callers to fail fast then to continue until a runtime error occurs at some indeterminate point in the future.

If your function has partially failed, but you feel that it has still produced some usable results, you can return a specific error type and document this case in the function comment.

Given the above, I would say you can do one of three things:

Return the successful result and also a specific error (which you document in the function comment). The caller can check for this error first, like so:

res, err := myLib.MakeHTTPCall()
if err == myLib.RequestBodyCloseErr {
   // do what you need to in case of this error
} else if err != nil {
   // general error handling
}

A second option is to return "" (the zero value for string) and the error from resp.Body.Close. In this case you are choosing to throw away the work the function has done up to this point.

The final option, if you don't want to stop processing for the error and you don't care enough about it to return it to the caller (I'm assuming this code is for you and your team and not some public library - if it's for a library the first two options would be better IMO.) is to simply log the error and return the successful result and nil.

Which option you choose is going to depend on how serious you think this error is. It also depends on how likely you think it is that this error will actually happen in production.

Improve this answer

8 Comments

It this convention documented anywhere? Any style-guides or any GH issues/PRs to follow? So what would you suggest in this case? Return non-nil value without error or non-nil error without value?
I don't think the "return zero value" convention is documented anywhere. I think this decision is made on a project-by-project or company-by-company basis. In your case I'd say you could do one of A. return 2 valid values and document that this can happen in the func's comment (you should return a specific error in this case so the caller can check for that before checking err != nil). B. Return "" and the error (you lose the successful result). C. Log the error and return the successful result and nil. It's up to you which one you prefer.
Let me edit my answer to have that information since I think my original answer missed the point of your question a bit (what you can do in this specific case)
Thanks. You mentioned "return zero value convention". What about "check error first and if error is not nil ignore other values" convention. Is it documented anywhere?
To rephrase more succinctly - if you receive an error for which it is not documented that the other return values are usable, and you continue with the returned values anyway - your code is depending on undefined behavior.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.