search

mgutz / dat

Go Postgres Data Access Toolkit
Other
610 stars 62 forks source link

Avoid use of panic #34

Open vcabbage opened 8 years ago

vcabbage commented 8 years ago

I have some tests that verify my application behaves properly when there are database issues. I found that on head these tests not only fail but end the test run due to the call to a call to logger.Fatal (https://github.com/mgutz/dat/blob/12949dfa90b45a1a62e6227fc1607c4fa1a3274c/sqlx-runner/db.go#L51), which eventually calls panic in logxi.

It's recommended to avoid exposing panics to consumer of a package and instead return an error so the consumer can choose how to deal with it. https://github.com/golang/go/wiki/PanicAndRecover#usage-in-a-package

On a related note, I noticed there are also some uses of log.Fatalf in sqlx-runner/db.go and sqlx-runner/tx.go. These call os.Exit(1) and should be avoid for similar reasons.

mgutz commented 8 years ago

Those are intentional. The first link's code ensures that interpolation is not used on a database where escape sequences are allowed in strings. The other two calls only occur in development mode when dat.Strict is set to true and only when a transaction cannot be committed or rolled back. In production mode, the error is returned.

vcabbage commented 8 years ago

Understood, but why does it panic? The consumer may want to know about and handle the error.

heynemann commented 7 years ago

I love dat, but I'm 100% with @vcabbage. It's being somewhat annoying having to handle the panics/fatals in my code. It would be so much easier to just handle an error (and I believe more idiomatic as well, the standard library very rarely panics).

Even so, thank you very much @mgutz for the awesome library.

mgutz commented 7 years ago

Which version are you using? V1 or V2?

heynemann commented 7 years ago

V1.

mgutz commented 7 years ago

Most of the panics have been converted to errors in V2. A proper package without panics requires a moderate refactor and version bump. That's down the future when I find time.