Repository: gobuffalo/pop (go)

Issue:
bug: transaction panic recovery swallows panic message
### Description

#### Description

When using transactions, any panic that occurs in the function passed to `pop.Transaction` is recovered and swallowed. The recovered error is checked for `nil`, but otherwise ignored. The error returned from the transaction is simply `fmt.Errorf("transaction was rolled back due to inner panic")`. There's no way to figure out what the panic was, because the original error is not wrapped, nor is it logged. This is especially problematic when using `popmw`, because _every_ request is wrapped in a transaction automatically, and now ~all panics are swallowed and undiagnosable.

#### Expected Behavior

The recovered error should be logged and/or wrapped by the returned error.

#### Actual Behavior

The recovered error is swallowed entirely. The returned generic error does generally ensure the request fails with a 500 response, but it's basically impossible to tell what caused the issue.

### To Reproduce

Use `app.User(popmw.Transaction(models.DB)`, then add a panic to a handler. Observe that the logs contain "transaction was rolled back due to inner panic", but not the details of the panic.

### Additional Context

_No response_

Pull request:
do not swallow panic when rolling back inner panic to prevent connection leak
This PR makes `Transaction()` not swallow inner panic when rolling back inner panic to prevent connection leak. (followup for #748, #775)

fixes #793

With this configuration,

```go
app.GET("/panic", func(c buffalo.Context) error {
        panic("panic message")
})
```

Previously we got a generic error but no information about the actual panic:

```
[POP] 2022/11/19 12:27:13 sql - BEGIN Transaction --- (conn=tx-2490861061155653040, tx=2490861061155653040)
[POP] 2022/11/19 12:27:13 sql - ROLLBACK Transaction (inner function panic) --- (conn=tx-2490861061155653040, tx=2490861061155653040)
ERRO[2022-11-19T12:27:13+09:00] transaction was rolled back due to inner panic conn=tx-2490861061155653040 db=0s params="{}" request_id=f3afa8520e74468e3126-605aa30f89aa98d921a6 status=500 tx=2490861061155653040
INFO[2022-11-19T12:27:13+09:00] /panic/ conn=tx-2490861061155653040 db=0s duration=4.668962ms human_size="33 kB" method=GET params="{}" path=/panic/ request_id=f3afa8520e74468e3126-605aa30f89aa98d921a6 size=33278 status=500 tx=2490861061155653040
```

now, the panic message will be handled by Buffalo's panic handler (or the application using Pop just gets the panic)

```
[POP] 2022/11/19 12:29:19 sql - BEGIN Transaction --- (conn=tx-6882700540045613063, tx=6882700540045613063)
[POP] 2022/11/19 12:29:19 sql - ROLLBACK Transaction (inner function panic) --- (conn=tx-6882700540045613063, tx=6882700540045613063)
ERRO[2022-11-19T12:29:19+09:00] panic message conn=tx-6882700540045613063 db=0s params="{}" request_id=d03cd4eaaeb706091e72-bd2ebc6009256e976b7e status=500 tx=6882700540045613063
INFO[2022-11-19T12:29:19+09:00] /panic/ conn=tx-6882700540045613063 db=0s duration=4.387724ms human_size="33 kB" method=GET params="{}" path=/panic/ request_id=d03cd4eaaeb706091e72-bd2ebc6009256e976b7e size=33245 status=500 tx=6882700540045613063
```

Developers can use https://github.com/gobuffalo/events/ for more details:

```go
func init() {
    events.Listen(func(e events.Event) {
        switch e.Kind {
        case events.ErrPanic:
            fmt.Println("panic error:", e.Error)
            if pl, ok := e.Payload["data"].(map[string]interface{}); ok {
                fmt.Println("panic stacktrace:", pl["stacktrace"])
            }
        }
    }) 
}
```

(`Payload` has a bug having additional "data", and will be fixed soon)

The above code will print the following:

```
panic error: panic message
panic stacktrace: goroutine 66 [running]:
runtime/debug.Stack()
        /opt/google/go/src/runtime/debug/stack.go:24 +0x65
github.com/gobuffalo/buffalo.(*App).PanicHandler.func1.1()
        /home/sio4/go/github.com/gobuffalo/buffalo/errors.go:98 +0x1e5
panic({0xbc8760, 0x1345270})
        /opt/google/go/src/runtime/panic.go:1038 +0x215
github.com/gobuffalo/pop/v6.(*Connection).Transaction.func1.1()
        /home/sio4/go/github.com/gobuffalo/pop/connection.go:173 +0xc5
panic({0xbc8760, 0x1345270})
        /opt/google/go/src/runtime/panic.go:1038 +0x215
coco/actions.App.func1.6({0xc000228001, 0xc000322f60})
        /home/sio4/git/bt/coco/actions/app.go:94 +0x27
```

Patch:
diff --git a/connection.go b/connection.go
index 9974fbd..a1fd077 100644
--- a/connection.go
+++ b/connection.go
@@ -168,9 +168,9 @@ func (c *Connection) Transaction(fn func(tx *Connection) error) error {
 				txlog(logging.SQL, cn, "ROLLBACK Transaction (inner function panic) ---")
 				dberr = cn.TX.Rollback()
 				if dberr != nil {
-					err = fmt.Errorf("database error while inner panic rollback: %w", dberr)
+					txlog(logging.Error, cn, "database error while inner panic rollback: %w", dberr)
 				}
-				err = fmt.Errorf("transaction was rolled back due to inner panic")
+				panic(ex)
 			}
 		}()
 
diff --git a/logger.go b/logger.go
index b1dd1ff..8a24224 100644
--- a/logger.go
+++ b/logger.go
@@ -50,30 +50,35 @@ var txlog = func(lvl logging.Level, anon interface{}, s string, args ...interfac
 		} else {
 			s = fmt.Sprintf("%s - %s", lvl, s)
 		}
-
-		connID := ""
-		txID := 0
-		switch typed := anon.(type) {
-		case *Connection:
-			connID = typed.ID
-			if typed.TX != nil {
-				txID = typed.TX.ID
-			}
-		case *Tx:
-			txID = typed.ID
-		case store:
-			tx, err := typed.Transaction()
-			if err == nil {
-				txID = tx.ID
-			}
-		}
-		s = fmt.Sprintf("%s (conn=%v, tx=%v)", s, connID, txID)
 	} else {
 		s = fmt.Sprintf(s, args...)
 		s = fmt.Sprintf("%s - %s", lvl, s)
 	}
+
+	connID := ""
+	txID := 0
+	switch typed := anon.(type) {
+	case *Connection:
+		connID = typed.ID
+		if typed.TX != nil {
+			txID = typed.TX.ID
+		}
+	case *Tx:
+		txID = typed.ID
+	case store:
+		tx, err := typed.Transaction()
+		if err == nil {
+			txID = tx.ID
+		}
+	}
+
+	if connID != "" || txID != 0 {
+		s = fmt.Sprintf("%s (conn=%v, tx=%v)", s, connID, txID)
+	}
+
 	if Color {
 		s = color.YellowString(s)
 	}
+
 	defaultStdLogger.Println(s)
 }


Test patch:
diff --git a/connection_test.go b/connection_test.go
index 90d5614..7826af7 100644
--- a/connection_test.go
+++ b/connection_test.go
@@ -138,10 +138,10 @@ func Test_Connection_Transaction(t *testing.T) {
 	})
 
 	t.Run("Panic", func(t *testing.T) {
-		err = c.Transaction(func(c *Connection) error {
-			panic("inner function panic")
+		r.PanicsWithValue("inner function panic", func() {
+			c.Transaction(func(c *Connection) error {
+				panic("inner function panic")
+			})
 		})
-		r.ErrorContains(err, "panic")
-		r.ErrorContains(err, "rolled back")
 	})
 }