database/sql: Why is errDBClosed not exported?

[chowey]

When I close a database connection and then try to execute a query on it, I get errDBClosed defined here:

http://golang.org/src/database/sql/sql.go#L630

Why is this variable not exported? It can be useful to do something like this:

_, err := db.Exec(`SELECT something`)
if err == sql.ErrDBClosed {
    return nil
}

The cost of exporting this variable is nothing.

[adg]

Why should it be exported? What would you do differently?

[chowey]

I would use it like in my example above.

[parkr]

This would be helpful in error handling.

[johto]

IME most packages don't export their errCloseds, and this issue has been brought up before, e.g. #4373.

[adg]

@chowey Why would you return nil instead of an error if the select fails? I'm looking for a bit more context, if you don't mind.

[chowey]

I'll explain my specific use case, so it is not so abstract.

I run a cooking process periodically on a separate goroutine. Occasionally this fails because of transaction locks, etc. My default behavior is to simply log the error and then retry during the next cook period.

When the program is asked to stop, I issue a db.Close(). This immediately causes any in-progress cooking process to fail with an error. The easy and robust way to deal with this is that the cooker understands this error message, stops its timer, does some fast cleanup and returns on its goroutine.

The current way I deal with it is to wrap db.Close() to save the "closed" state and then specifically check for the "closed" state when the cooker has an error, so it cleans up and exits gracefully at that point. This is a little more overhead but it adds up in complexity when I have multiple cookers.

Specifically I wouldn't return a nil instead of an error. The code looks more like this (or would look like this if errDBClosed was exported):

go func() {
    for {
        time.Sleep(time.Minute)
        _, err := db.Exec(`UPDATE something`)
        if err == sql.ErrDBClosed {
            // some fast cleanup code
            return
        } else {
            log.Printf("%v", err)
        }
    }
}()

[adg]

My reading of the database/sql code might be wrong, but I think you can end up in a state where the database is closed because of an error, rather than you gracefully shutting it down. I think relying on db.Close as such a signal is probably not the right way to go.

You might try something like this, where all your workers can select on a timer or a quit channel, and your controller can signal shutdown with close(quit). That will shut your workers down immediately, instead of after the timeout, and you can also put a check in the select statement error handling to see whether you should report the error (I'd say you should anyway).

var quit chan struct{}

go func() {
    for {
        select {
        case <-quit:
            // some fast cleanup code
            return
        case <-time.After(time.Minute):
        }
        if _, err := db.Exec(`UPDATE something`); err != nil {
            log.Printf("%v", err)
        }
    }
}()

[chowey]

I'm certainly not arguing that there is no other way around this problem.

My question simply was, why is errDBClosed not exported?

Edit: Specifically, it seems to have been created with this commit:

41c5d8d#diff-ab2cf91bbf4757084fddc5afa8bf8f07R602

It looks like prior to this, the error was being reported using errors.New and the author of this commit decided to make it a variable because he uses it in two places instead of one. Probably it was done because it was minimal impact.

[rsc]

It is not exported because code is not supposed to check for it.