oschwald / maxminddb-golang

MaxMind DB Reader for Go
ISC License
615 stars 101 forks source link

internal errors should be wrap or encapsulated? #93

Closed lingfengChen03 closed 3 months ago

lingfengChen03 commented 2 years ago

in the process of using Sometimes we needs to determine if the status of mmdb is closed or not.

errors.New("cannot call Lookup on a closed database")

Now that I'm out there judging strings, there should be a more elegant way to do it? Looking forward to the author's recovery~

oschwald commented 2 years ago

Although I would accept a PR to make a more specific error type for this case, could you expand a bit on how you end up in the situation where you don't know if the reader is closed or not? In my own use, this would generally be indicative of a programming bug in the application.

lingfengChen03 commented 2 years ago

Although I would accept a PR to make a more specific error type for this case, could you expand a bit on how you end up in the situation where you don't know if the reader is closed or not? In my own use, this would generally be indicative of a programming bug in the application.

Thank you for your reply~ In the code, there is a timer that updates the mmdb regularly, meaning that one closes the old mmdb and opens the new one, so if another read-goroutine reads it, it may return an exception, so I want to get the current mmdb status for the subsequent logic.

    go func() {
        for range ticker.C {
            if MmdbReader != nil {
                MmdbReader.Close()
            }
            // init mmdb
            MmdbReader, err = maxminddb.Open(Conf.MaxmindDb)
                         ....
        }
    }()
oschwald commented 2 years ago

I see. For what it is worth, in situations like this one, I atomically replace the reader and rely on the finalizer to eventually close the old one. If you don't want to rely on the finalizer, you could also start a new goroutine to close it after an appropriate amount of time has passed for your use case.

oschwald commented 3 months ago

Closing as I don't think this is an error that the user should expect and making it a sentinel error does not seem appropriate.