tablet/gorm
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

transaction blocks: don't swallow panics

Browse Source 
This improves upon #2767.

Previously, the code would swallow any panics, which isn't ideal;
panic is intended to be used when a critical error arises,
where the process should fail fast instead of trying to limp along.

This now defers the any recovery (if desired) to the client code.
This commit is contained in:
Charles Strahan 2019-11-22 15:09:48 -08:00
parent 59408390c2
commit 4495b28c87
2 changed files with 24 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
main.go
View File

@ -528,12 +528,12 @@ func (s *DB) Debug() *DB {
// Transaction start a transaction as a block, // Transaction start a transaction as a block,
// return error will rollback, otherwise to commit. // return error will rollback, otherwise to commit.
func (s *DB) Transaction(fc func(tx *DB) error) (err error) { func (s *DB) Transaction(fc func(tx *DB) error) (err error) {
panicked := true
tx := s.Begin() tx := s.Begin()
defer func() { defer func() {
if r := recover(); r != nil { // Make sure to rollback when panic, Block error or Commit error
err = fmt.Errorf("%s", r) if panicked || err != nil {
tx.Rollback() tx.Rollback()
return
} }
}() }()
@ -543,10 +543,7 @@ func (s *DB) Transaction(fc func(tx *DB) error) (err error) {
err = tx.Commit().Error err = tx.Commit().Error
} }
// Makesure rollback when Block error or Commit error panicked = false
if err != nil {
tx.Rollback()
}
return return
} }

29
main_test.go
View File

@ -470,6 +470,15 @@ func TestTransaction(t *testing.T) {
} }
} }
func assertPanic(t *testing.T, f func()) {
defer func() {
if r := recover(); r == nil {
t.Errorf("The code did not panic")
}
}()
f()
}
func TestTransactionWithBlock(t *testing.T) { func TestTransactionWithBlock(t *testing.T) {
// rollback // rollback
err := DB.Transaction(func(tx *gorm.DB) error { err := DB.Transaction(func(tx *gorm.DB) error {
@ -511,17 +520,19 @@ func TestTransactionWithBlock(t *testing.T) {
} }
// panic will rollback // panic will rollback
DB.Transaction(func(tx *gorm.DB) error { assertPanic(t, func() {
u3 := User{Name: "transcation-3"} DB.Transaction(func(tx *gorm.DB) error {
if err := tx.Save(&u3).Error; err != nil { u3 := User{Name: "transcation-3"}
t.Errorf("No error should raise") if err := tx.Save(&u3).Error; err != nil {
} t.Errorf("No error should raise")
}
if err := tx.First(&User{}, "name = ?", "transcation-3").Error; err != nil { if err := tx.First(&User{}, "name = ?", "transcation-3").Error; err != nil {
t.Errorf("Should find saved record") t.Errorf("Should find saved record")
} }
panic("force panic") panic("force panic")
})
}) })
if err := DB.First(&User{}, "name = ?", "transcation").Error; err == nil { if err := DB.First(&User{}, "name = ?", "transcation").Error; err == nil {