I really do not like code like
defer func() {
if cerr := f.Close(); cerr != nil && err == nil {
err = cerr
}
}()
My problem with this code is:
1) It is ugly
2) It is too clever
3) It is not clear what the code actually does
4) It is a trap for maintainers of the code
This code _seems_ to pass any error returned by f.Close() to the enclosing
function.
But it only works if the function has a named return parameter. If somebody
later
changes the function to return unnamed values, this defer will silently
fail.
This happens all the time out in the wild.
(Check out
https://github.com/blevesearch/bleve/blob/369ad22b9fefdadb42b9367193ef7a73b9e5f4ff/search/searcher/search_regexp.go#L54
for instance).
So my advice would be
1) Keep the code simple
2) Don't use defer in places where it leads to convoluted code.
3) It is often OK ignore the error returned by f.Close()
e.g. if f is a Reader, a bytes.Buffer, or a http.ResponseWriter.
- amnon
On Saturday, 30 January 2021 at 09:58:56 UTC Uli Kunitz wrote:
> You need to think about how important it is for the user of your function
> or method to know about the Close error. Usually when a function closes a
> resource it also acquires it, so your caller cannot do anything to fix the
> closure. So I report only the first error.
>
> The question that is more difficult to answer is, should errors of Close
> functions be reported, if there is no error in the function. The typical
> defer f.Close() doesn't do it. If your code is mission critical you should
> do it, but it will require something like:
>
> defer func() {
> if cerr := f.Close(); cerr != nil && err == nil {
> err = cerr
> }
> }()
>
> The variable err must be of course a named return value. I would recommend
> to do it, if this is mission-critical code. Another approach might be the
> following,
>
> f , err := os.Open(....)
> if err {
> return err
> }
> defer f.Close()
> // work with f
> if err := f.Close(); err != nil {
> return err
> }
>
> This requires that you can call Close twice and the second Call is
> essentially a no-op potentially returning an error. You must also ensure
> that all successful path end at the f.Close call.
> On Saturday, January 30, 2021 at 10:10:55 AM UTC+1 josvazg wrote:
>
>> Is there some idiomatic or well known way to deal with extra errors when
>> you are already recovering from or reporting an error condition?
>>
>> The typical case is, you hit an error and need to release resources
>> before returning it. But the resources themselves may fail when closing
>> them.
>>
>> One (frequent) way seems to be to defer the freeing/closing so the
>> resource will be freed on exit of the call no matter what. But that way you
>> could be swallowing resource closing errors, which could lead to long
>> debugging sessions in production.
>>
>> Another way is to have an adhoc close & report wrapper function to use
>> when deferring resource closing call that may fail. The wrapper can log the
>> fact that a closing error happened while reporting some other error, then
>> choose to return one or the other up the stack. But that can make the code
>> ugly and difficult to read. Or is there a way to wrap 2 errors at once
>> somehow?
>>
>> Are there better ways to handle this situations?
>>
>> Thanks,
>>
>> Jose
>>
>
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/golang-nuts/1bd61f42-4dec-43c6-b21b-ec3962ef8d2fn%40googlegroups.com.
