Badger: Errors in the API

Created on 15 May 2017  路  11Comments  路  Source: dgraph-io/badger

I'm trying badger as a replacement for github.com/syndtr/goleveldb and one thing that hit me immediately is that there are no errors in the badger Go API. Any error will just exit the process through log.Fatalf. Is that a design decision or just temporary?

Most helpful comment

badger.NewKV("non-existent-dir/foo")

crashes my program. That's not acceptable. Opening a non-existent file is not an internal error, it's a user error which my program will want to relay to the user in an arbitrary way.

All 11 comments

I see that there is a way to get errors when using Entry. Would still be nice to have a version of NewKV that doesn't exit the program.

log.Fatals only happen when Badger itself causes issues. External input won't cause it. In fact, these Fatals are really great at catching bugs and making the library robust.

We send errors back when justified, for e.g. when doing CAS operations.

badger.NewKV("non-existent-dir/foo")

crashes my program. That's not acceptable. Opening a non-existent file is not an internal error, it's a user error which my program will want to relay to the user in an arbitrary way.

We are adding basic checks for the directory to ease usage for the newcomers, see #29.
Overall, we decided that it's library user's responsibility to provide a writable directory for badger for the whole duration of using KV.

There is also a separate thread for disk errors: #28

Any operation that could fail, under any conditions, should return an error.

we decided that it's library user's responsibility to provide a writable directory

Sure, you can hope the user provides a writeable directory, but the fact remains that if they give you a directory, and you can't write to it, you need to tell them they failed on their end of the contract.

I see your point, you are right. We can do much better error handling.

Besides the pragmatic concerns, it violates go conventions to panic across a package boundary.

Yup. Agree with all the concerns here. We have started to surface errors via the API calls (see https://github.com/dgraph-io/badger/commit/31c0bd2026fc93f727422122f0e087bc1961f78f). We'll do a sweep over the next couple of weeks over the entire code base. And try to remove all calls to y.Checkf, if possible.

The biggest hurdle would be to deal with background tasks, and we'll need to build mechanisms to deal graciously with them. I'll keep you updated on this thread.

Good to hear, looking forward to using the library.

See changes: https://github.com/dgraph-io/badger/commit/55c350dd48002b721e305a08f6ee585051453dfe and https://github.com/dgraph-io/badger/commit/7610c2f1b92f0107e4f37e37e24aa82a13839284. These surface error handling all the way from disk to public APIs. They should address the concerns raised in this issue.

Closing the issue. Feel free to reopen if some API is still unhandled.

Excellent. I will probably switch our usage of BoltDB to Badger, since we are in the early stages of our project as well.

Was this page helpful?
0 / 5 - 0 ratings