Then there should only be a single err variable, and the address should not
change. Not sure why it isn’t working.... are you sure you are not in a code
block that is causing a shadowing of err?
> On Dec 3, 2018, at 4:54 PM, Ben Hoyt <benh...@gmail.com> wrote:
>
> Ah, quite right. That's what comes of trying to modify the code snippet from
> our actual code on the fly. It was more like:
>
> t, err := InsertFoo(tx)
>
> -Ben
>
>
>> On Mon, Dec 3, 2018, 5:50 PM Robert Engels <reng...@ix.netcom.com wrote:
>> How can you write this
>>
>> err := InsertFoo(tx)
>>
>> Don’t you get no new variables defined error here?
>>
>>> On Dec 3, 2018, at 3:53 PM, Ben Hoyt <benh...@gmail.com> wrote:
>>>
>>> Hi folks,
>>>
>>> We found some subtle bugs in our db transaction code for handling
>>> commits/rollbacks. Here's the pattern we were using (not real, but shows
>>> the issue):
>>>
>>> func DoTwoThings() error {
>>> tx, err := db.Begin()
>>> if err != nil {
>>> return err
>>> }
>>> // commit or rollback the transaction before we return
>>> defer tx.Close(&err)
>>>
>>> err := InsertFoo(tx)
>>> if err != nil {
>>> return err
>>> }
>>> if _, err := UpdateBar(tx); err != nil {
>>> return err
>>> }
>>> return nil
>>> }
>>>
>>> The problem is there's a subtle but potentially quite bad bug with this
>>> usage pattern -- if the InsertFoo succeeds but UpdateBar fails, the first
>>> "err" variable will be nil, so the deferred tx.Close() will COMMIT the
>>> transaction rather than ROLLBACK, and the database will be in an
>>> inconsistent state.
>>>
>>> The code above is a bit contrived, and you can easily fix it by moving the
>>> "_, err := UpdateBar()" outside of the if so it's referring to the same
>>> "err" variable, but it's very easy to miss and get it wrong. So we decided
>>> it was a bad pattern and started thinking about the best way to fix.
>>>
>>> One idea is a RollbackUnlessCommitted() function which you can defer, and
>>> then you call Commit() once manually (stolen from gocraft/dbr):
>>>
>>> func DoTwoThings() error {
>>> tx, err := db.Begin()
>>> if err != nil {
>>> return err
>>> }
>>> defer tx.RollbackUnlessCommitted()
>>>
>>> err := InsertFoo(tx)
>>> if err != nil {
>>> return err
>>> }
>>> if _, err := UpdateBar(tx); err != nil {
>>> return err
>>> }
>>> tx.Commit()
>>> return nil
>>> }
>>>
>>> Another idea is to create a "Transact" function which takes an anonymous
>>> function and does all the transaction handling:
>>>
>>> func (db *DatabaseImpl) Transact(txFunc func() error) (err error) {
>>> tx, err := db.Begin()
>>> if err != nil {
>>> return
>>> }
>>> defer func() {
>>> if p := recover(); p != nil {
>>> tx.Rollback()
>>> panic(p) // re-throw panic after Rollback
>>> } else if err != nil {
>>> tx.Rollback() // err is non-nil; don't change it
>>> } else {
>>> err = tx.Commit() // err is nil; if Commit returns error update
>>> err
>>> }
>>> }()
>>> err = txFunc(tx)
>>> return err
>>> }
>>>
>>> And then the DoTwoThings function becomes:
>>>
>>> func DoTwoThings() error {
>>> return db.Transact(func() error) {
>>> err := InsertFoo(tx)
>>> if err != nil {
>>> return err
>>> }
>>> if _, err := UpdateBar(tx); err != nil {
>>> return err
>>> }
>>> })
>>> }
>>>
>>> I think the second is probably safer and nicer, but it's slightly awkward
>>> in that it requires an extra level of indentation. Still, awkward is better
>>> than buggy.
>>>
>>> Does anyone else have a better pattern for this kind of thing, or feedback
>>> on the above?
>>>
>>> -Ben
>>>
>>> --
>>> 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.
>>> For more options, visit https://groups.google.com/d/optout.
>
> --
> 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.
> For more options, visit https://groups.google.com/d/optout.
--
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.
For more options, visit https://groups.google.com/d/optout.
